Reads network quality provided by the NQE and compares it

with field trial parameters to determine if the network is
prohibitively slow.

Hysteresis is added so that at least k seconds transpire
before the state of the network (prohibitively slow, or
not) is determined again. k is a parameter of the field
trial.

BUG=470596

Review URL: https://codereview.chromium.org/1153543016

Cr-Commit-Position: refs/heads/master@{#333609}

5 files changed, 132 insertions, 25 deletions

diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
index 08cf86f..aab7569 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
@@ -196,9 +196,12 @@ DataReductionProxyConfig::DataReductionProxyConfig(
       net_log_(net_log),
       configurator_(configurator),
       event_creator_(event_creator),
-      auto_lofi_minimum_rtt_(base::TimeDelta()),
-      auto_lofi_maximum_kbps_(0),
-      auto_lofi_hysteresis_(base::TimeDelta()),
+      auto_lofi_minimum_rtt_(base::TimeDelta::Max()),
+      auto_lofi_maximum_kbps_(UINT64_MAX),
+      auto_lofi_hysteresis_(base::TimeDelta::Max()),
+      network_quality_last_updated_(base::TimeTicks()),
+      network_prohibitively_slow_(false),
+      connection_type_(net::NetworkChangeNotifier::GetConnectionType()),
       lofi_status_(LOFI_STATUS_TEMPORARILY_OFF) {
   DCHECK(configurator);
   DCHECK(event_creator);
@@ -370,19 +373,46 @@ bool DataReductionProxyConfig::AreProxiesBypassed(
 }
 
 bool DataReductionProxyConfig::IsNetworkQualityProhibitivelySlow(
-    const net::NetworkQualityEstimator* network_quality_estimator) const {
+    const net::NetworkQualityEstimator* network_quality_estimator) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!network_quality_estimator)
     return false;
 
+  // True iff network type changed since the last call to
+  // IsNetworkQualityProhibitivelySlow(). This call happens only on main frame
+  // requests.
+  bool network_type_changed = false;
+  if (net::NetworkChangeNotifier::GetConnectionType() != connection_type_) {
+    connection_type_ = net::NetworkChangeNotifier::GetConnectionType();
+    network_type_changed = true;
+  }
+  // Return the cached entry if the last update was within the hysteresis
+  // duration and if the connection type has not changed.
+  if (!network_type_changed && !network_quality_last_updated_.is_null() &&
+      base::TimeTicks::Now() - network_quality_last_updated_ <=
+          auto_lofi_hysteresis_) {
+    return network_prohibitively_slow_;
+  }
+
+  network_quality_last_updated_ = base::TimeTicks::Now();
+
   net::NetworkQuality network_quality =
       network_quality_estimator->GetEstimate();
   // TODO(tbansal): Set |network_prohibitively_slow| based on medians
   // provided by NetworkQualityEstimator API and field trial parameters.
-  // Also, ensure that state of network is not changed more than once within
-  // the hysteresis period.
-  return false;
+
+  // Network is prohibitvely slow if either the downlink bandwidth is too low
+  // or the RTT is too high.
+  if ((network_quality.peak_throughput_kbps > 0 &&
+       network_quality.peak_throughput_kbps <= auto_lofi_maximum_kbps_) ||
+      (network_quality.fastest_rtt > base::TimeDelta() &&
+       network_quality.fastest_rtt >= auto_lofi_minimum_rtt_)) {
+    network_prohibitively_slow_ = true;
+  } else {
+    network_prohibitively_slow_ = false;
+  }
+  return network_prohibitively_slow_;
 }
 
 bool DataReductionProxyConfig::IsIncludedInLoFiEnabledFieldTrial() const {
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
index 074f913..b015f06 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
@@ -277,8 +277,7 @@ class DataReductionProxyConfig
                            TestMaybeDisableIfVPNTrialDisabled);
   FRIEND_TEST_ALL_PREFIXES(DataReductionProxyConfigTest,
                            TestMaybeDisableIfVPNTrialEnabled);
-  FRIEND_TEST_ALL_PREFIXES(DataReductionProxyConfigTest,
-                           PopulateAutoLoFiParams);
+  FRIEND_TEST_ALL_PREFIXES(DataReductionProxyConfigTest, AutoLoFiParams);
 
   // NetworkChangeNotifier::IPAddressObserver:
   void OnIPAddressChanged() override;
@@ -340,7 +339,7 @@ class DataReductionProxyConfig
   // |network_quality_estimator| may be NULL.
   // Virtualized for unit testing.
   virtual bool IsNetworkQualityProhibitivelySlow(
-      const net::NetworkQualityEstimator* network_quality_estimator) const;
+      const net::NetworkQualityEstimator* network_quality_estimator);
 
   // Returns true only if Lo-Fi "q=low" header should be added to the Chrome
   // Proxy header based on the value of |lofi_status|.
@@ -389,6 +388,17 @@ class DataReductionProxyConfig
   // duration shorter than |auto_lofi_hysteresis_|.
   base::TimeDelta auto_lofi_hysteresis_;
 
+  // Time when the network quality was last updated.
+  base::TimeTicks network_quality_last_updated_;
+
+  // True iff the network was determined to be prohibitively slow when the
+  // network quality was last updated (most recent main frame request).
+  bool network_prohibitively_slow_;
+
+  // Set to the connection type reported by NetworkChangeNotifier when the
+  // network quality was last updated (most recent main frame request).
+  net::NetworkChangeNotifier::ConnectionType connection_type_;
+
   // Current Lo-Fi status.
   // The value changes only on main frame load.
   LoFiStatus lofi_status_;
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
index e5714e4..5c6ed93 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
@@ -52,7 +52,7 @@ TestDataReductionProxyConfig::~TestDataReductionProxyConfig() {
 }
 
 bool TestDataReductionProxyConfig::IsNetworkQualityProhibitivelySlow(
-    const net::NetworkQualityEstimator* network_quality_estimator) const {
+    const net::NetworkQualityEstimator* network_quality_estimator) {
   return network_quality_prohibitively_slow_;
 }
 
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
index d31ed9a..efb9070 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
@@ -80,8 +80,7 @@ class TestDataReductionProxyConfig : public DataReductionProxyConfig {
   void SetNetworkProhibitivelySlow(bool network_quality_prohibitively_slow);
 
   bool IsNetworkQualityProhibitivelySlow(
-      const net::NetworkQualityEstimator* network_quality_estimator)
-      const override;
+      const net::NetworkQualityEstimator* network_quality_estimator) override;
 
   net::NetworkInterfaceList* interfaces() {
     return network_interfaces_.get();
@@ -133,7 +132,7 @@ class MockDataReductionProxyConfig : public TestDataReductionProxyConfig {
   MOCK_METHOD2(SecureProxyCheck,
                void(const GURL& secure_proxy_check_url,
                     FetcherResponseCallback fetcher_callback));
-  MOCK_CONST_METHOD1(
+  MOCK_METHOD1(
       IsNetworkQualityProhibitivelySlow,
       bool(const net::NetworkQualityEstimator* network_quality_estimator));
   MOCK_CONST_METHOD0(IsIncludedInLoFiEnabledFieldTrial, bool());
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
index b1e6316..26f726d 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
+#include "base/strings/safe_sprintf.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h"
@@ -19,6 +20,8 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
 #include "components/variations/variations_associated_data.h"
+#include "net/base/network_quality.h"
+#include "net/base/network_quality_estimator.h"
 #include "net/http/http_status_code.h"
 #include "net/log/test_net_log.h"
 #include "net/proxy/proxy_server.h"
@@ -1213,30 +1216,95 @@ TEST_F(DataReductionProxyConfigTest, LoFiStatusTransition) {
   }
 }
 
-TEST_F(DataReductionProxyConfigTest, PopulateAutoLoFiParams) {
+// Overrides net::NetworkQualityEstimator::GetEstimate() for testing purposes.
+class TestNetworkQualityEstimator : public net::NetworkQualityEstimator {
+ public:
+  TestNetworkQualityEstimator() : rtt_(base::TimeDelta()), kbps_(0) {}
+
+  ~TestNetworkQualityEstimator() override {}
+
+  net::NetworkQuality GetEstimate() const override {
+    return net::NetworkQuality(rtt_, 0.0, kbps_, 0.0);
+  }
+
+  void SetRtt(base::TimeDelta rtt) { rtt_ = rtt; }
+
+  void SetKbps(uint64_t kbps) { kbps_ = kbps; }
+
+ private:
+  base::TimeDelta rtt_;
+  uint64_t kbps_;
+};
+
+TEST_F(DataReductionProxyConfigTest, AutoLoFiParams) {
+  DataReductionProxyConfig config(nullptr, nullptr, configurator(),
+                                  event_creator());
   variations::testing::ClearAllVariationParams();
   std::map<std::string, std::string> variation_params;
-  variation_params["rtt_msec"] = "120";
+
+  int rtt_msec = 120;
+  char rtt[20];
+  base::strings::SafeSPrintf(rtt, "%d", rtt_msec);
+  variation_params["rtt_msec"] = rtt;
+
   variation_params["kbps"] = "240";
-  variation_params["hysteresis_period_seconds"] = "360";
+
+  int hysteresis_sec = 360;
+  char hysteresis[20];
+  base::strings::SafeSPrintf(hysteresis, "%d", hysteresis_sec);
+  variation_params["hysteresis_period_seconds"] = hysteresis;
+
   variation_params["spurious_field"] = "480";
 
   ASSERT_TRUE(variations::AssociateVariationParams(
       DataReductionProxyParams::GetLoFiFieldTrialName(), "Enabled",
       variation_params));
-  EXPECT_CALL(*config(), IsIncludedInLoFiEnabledFieldTrial())
-      .WillRepeatedly(testing::Return(true));
 
   base::FieldTrialList field_trial_list(nullptr);
   base::FieldTrialList::CreateFieldTrial(
       DataReductionProxyParams::GetLoFiFieldTrialName(), "Enabled");
 
-  config()->PopulateAutoLoFiParams();
-
-  EXPECT_EQ(config()->auto_lofi_minimum_rtt_,
-            base::TimeDelta::FromMilliseconds(120));
-  EXPECT_EQ(config()->auto_lofi_maximum_kbps_, 240U);
-  EXPECT_EQ(config()->auto_lofi_hysteresis_, base::TimeDelta::FromSeconds(360));
+  config.PopulateAutoLoFiParams();
+
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(rtt_msec),
+            config.auto_lofi_minimum_rtt_);
+  EXPECT_EQ(240U, config.auto_lofi_maximum_kbps_);
+  EXPECT_EQ(base::TimeDelta::FromSeconds(hysteresis_sec),
+            config.auto_lofi_hysteresis_);
+
+  TestNetworkQualityEstimator test_network_quality_estimator;
+
+  // RTT is higher than threshold. Network is slow.
+  test_network_quality_estimator.SetRtt(
+      base::TimeDelta::FromMilliseconds(rtt_msec + 1));
+  EXPECT_TRUE(config.IsNetworkQualityProhibitivelySlow(
+      &test_network_quality_estimator));
+
+  // Network quality improved. RTT is lower than the threshold. However,
+  // network should still be marked as slow because of hysteresis.
+  test_network_quality_estimator.SetRtt(
+      base::TimeDelta::FromMilliseconds(rtt_msec - 1));
+  EXPECT_TRUE(config.IsNetworkQualityProhibitivelySlow(
+      &test_network_quality_estimator));
+
+  // Change the last update time to be older than the hysteresis duration.
+  // Checking network quality afterwards should show that network is no longer
+  // slow.
+  config.network_quality_last_updated_ =
+      base::TimeTicks::Now() - base::TimeDelta::FromSeconds(hysteresis_sec + 1);
+  EXPECT_FALSE(config.IsNetworkQualityProhibitivelySlow(
+      &test_network_quality_estimator));
+
+  // Changing the RTT has no effect because of hysteresis.
+  test_network_quality_estimator.SetRtt(
+      base::TimeDelta::FromMilliseconds(rtt_msec + 1));
+  EXPECT_FALSE(config.IsNetworkQualityProhibitivelySlow(
+      &test_network_quality_estimator));
+
+  // Change in connection type changes the network quality despite hysteresis.
+  config.connection_type_ = net::NetworkChangeNotifier::CONNECTION_WIFI;
+  EXPECT_TRUE(config.IsNetworkQualityProhibitivelySlow(
+      &test_network_quality_estimator));
 }
 
 }  // namespace data_reduction_proxy