[DRP] Consistently use LoFi for an entire page

components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
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 146af5c18ede27f1acdf2b1450d4c143e75df2cf..ebec7281c67c8430d3a8432ec83ebbe551136201 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
@@ -25,6 +25,7 @@
 #include "net/proxy/proxy_server.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
@@ -79,10 +80,8 @@ bool FindProxyInList(const std::vector<net::ProxyServer>& proxy_list,
 
 // Following UMA is plotted to measure how frequently Lo-Fi state changes.
 // Too frequent changes are undesirable.
-void RecordAutoLoFiRequestHeaderStateChange(
-    net::NetworkChangeNotifier::ConnectionType connection_type,
-    bool previous_header_low,
-    bool current_header_low) {
+void RecordAutoLoFiRequestHeaderStateChange(bool previous_header_low,
+                                            bool current_header_low) {
   // Auto Lo-Fi request header state changes.
   // Possible Lo-Fi header directives are empty ("") and low ("q=low").
   // This enum must remain synchronized with the enum of the same name in
@@ -96,6 +95,9 @@ void RecordAutoLoFiRequestHeaderStateChange(
   };
 
   AutoLoFiRequestHeaderState state;
+  net::NetworkChangeNotifier::ConnectionType connection_type =
+      net::NetworkChangeNotifier::GetConnectionType();
+
   if (!previous_header_low) {
     if (current_header_low)
       state = AUTO_LOFI_REQUEST_HEADER_STATE_EMPTY_TO_LOW;
@@ -243,10 +245,11 @@ DataReductionProxyConfig::DataReductionProxyConfig(
       network_quality_last_updated_(base::TimeTicks()),
       network_prohibitively_slow_(false),
       connection_type_(net::NetworkChangeNotifier::GetConnectionType()),
-      lofi_status_(LOFI_STATUS_TEMPORARILY_OFF),
+      lofi_off_(false),
       last_main_frame_request_(base::TimeTicks::Now()),
       network_quality_at_last_main_frame_request_(
-          NETWORK_QUALITY_AT_LAST_MAIN_FRAME_REQUEST_UNKNOWN) {
+          NETWORK_QUALITY_AT_LAST_MAIN_FRAME_REQUEST_UNKNOWN),
+      previous_state_lofi_on_(false) {
   DCHECK(configurator);
   DCHECK(event_creator);
   if (params::IsLoFiDisabledViaFlags())
@@ -462,38 +465,6 @@ bool DataReductionProxyConfig::IsIncludedInLoFiControlFieldTrial() const {
          kControl;
 }
 
-LoFiStatus DataReductionProxyConfig::GetLoFiStatus() const {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return lofi_status_;
-}
-
-// static
-bool DataReductionProxyConfig::ShouldUseLoFiHeaderForRequests(
-    LoFiStatus lofi_status) {
-  switch (lofi_status) {
-    case LOFI_STATUS_OFF:
-    case LOFI_STATUS_TEMPORARILY_OFF:
-    case LOFI_STATUS_ACTIVE_CONTROL:
-    case LOFI_STATUS_INACTIVE_CONTROL:
-    case LOFI_STATUS_INACTIVE:
-      return false;
-    // Lo-Fi header can be used only if Lo-Fi is not temporarily off and either
-    // the user has enabled Lo-Fi through flags, or session is in Lo-Fi enabled
-    // group with network quality prohibitively slow.
-    case LOFI_STATUS_ACTIVE_FROM_FLAGS:
-    case LOFI_STATUS_ACTIVE:
-      return true;
-    default:
-      NOTREACHED() << lofi_status;
-  }
-  return false;
-}
-
-bool DataReductionProxyConfig::ShouldUseLoFiHeaderForRequests() const {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return ShouldUseLoFiHeaderForRequests(lofi_status_);
-}
-
 void DataReductionProxyConfig::PopulateAutoLoFiParams() {
   std::string field_trial = params::GetLoFiFieldTrialName();
 
@@ -763,7 +734,7 @@ void DataReductionProxyConfig::SecureProxyCheck(
 
 void DataReductionProxyConfig::SetLoFiModeOff() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  lofi_status_ = LOFI_STATUS_OFF;
+  lofi_off_ = true;
 }
 
 void DataReductionProxyConfig::RecordAutoLoFiAccuracyRate(
@@ -863,8 +834,25 @@ void DataReductionProxyConfig::RecordAutoLoFiAccuracyRate(
   }
 }
 
-void DataReductionProxyConfig::UpdateLoFiStatusOnMainFrameRequest(
-    bool user_temporarily_disabled_lofi,
+bool DataReductionProxyConfig::ShouldEnableLoFiMode(net::URLRequest* request) {
+  net::NetworkQualityEstimator* network_quality_estimator = nullptr;
+  if (request && request->context())
+    network_quality_estimator = request->context()->network_quality_estimator();
+
+  bool enable_lofi =
+      ShouldTurnOnLoFiOnMainFrameRequest(network_quality_estimator);
+
+  if (params::IsLoFiSlowConnectionsOnlyViaFlags() ||
+      IsIncludedInLoFiEnabledFieldTrial()) {
+    RecordAutoLoFiRequestHeaderStateChange(previous_state_lofi_on_,
+                                           enable_lofi);
+    previous_state_lofi_on_ = enable_lofi;
+  }
+
+  return enable_lofi;
+}
+
+bool DataReductionProxyConfig::ShouldTurnOnLoFiOnMainFrameRequest(
     const net::NetworkQualityEstimator* network_quality_estimator) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
@@ -882,77 +870,28 @@ void DataReductionProxyConfig::UpdateLoFiStatusOnMainFrameRequest(
   network_quality_at_last_main_frame_request_ =
       NETWORK_QUALITY_AT_LAST_MAIN_FRAME_REQUEST_UNKNOWN;
 
-  // If Lo-Fi has been permanently turned off, its status can't change.
-  if (lofi_status_ == LOFI_STATUS_OFF)
-    return;
-
-  // If the user has temporarily disabled Lo-Fi on a main frame request, it will
-  // remain disabled until next main frame request.
-  if (user_temporarily_disabled_lofi) {
-    switch (lofi_status_) {
-      // Turn off Lo-Fi temporarily (until next main frame request) if it was
-      // enabled from flags or because the session is in Lo-Fi enabled group.
-      case LOFI_STATUS_ACTIVE_FROM_FLAGS:
-      case LOFI_STATUS_ACTIVE:
-      case LOFI_STATUS_INACTIVE:
-        lofi_status_ = LOFI_STATUS_TEMPORARILY_OFF;
-        return;
-      // Lo-Fi is already temporarily off, so no need to change state.
-      case LOFI_STATUS_TEMPORARILY_OFF:
-      // If the current session does not have Lo-Fi switch, is not in Auto Lo-Fi
-      // enabled group and is in Auto Lo-Fi control group, then we do not need
-      // to temporarily disable Lo-Fi because it would never be used.
-      case LOFI_STATUS_ACTIVE_CONTROL:
-      case LOFI_STATUS_INACTIVE_CONTROL:
-        return;
-
-      default:
-        NOTREACHED() << "Unexpected Lo-Fi status = " << lofi_status_;
-    }
-  }
+  // If Lo-Fi has been turned off, its status can't change.
+  if (lofi_off_)
+    return false;
 
-  if (params::IsLoFiAlwaysOnViaFlags()) {
-    lofi_status_ = LOFI_STATUS_ACTIVE_FROM_FLAGS;
-    return;
-  }
+  if (params::IsLoFiAlwaysOnViaFlags())
+    return true;
 
   if (params::IsLoFiCellularOnlyViaFlags()) {
-    if (net::NetworkChangeNotifier::IsConnectionCellular(
-            net::NetworkChangeNotifier::GetConnectionType())) {
-      lofi_status_ = LOFI_STATUS_ACTIVE_FROM_FLAGS;
-      return;
-    }
-    lofi_status_ = LOFI_STATUS_TEMPORARILY_OFF;
-    return;
+    return net::NetworkChangeNotifier::IsConnectionCellular(
+        net::NetworkChangeNotifier::GetConnectionType());
   }
 
-  // Store the previous state of Lo-Fi, so that change in Lo-Fi status can be
-  // recorded properly. This is not needed for the control group, because it
-  // is only used to report changes in request headers, and the request headers
-  // are never modified in the control group.
-  LoFiStatus previous_lofi_status = lofi_status_;
-
   if (params::IsLoFiSlowConnectionsOnlyViaFlags() ||
-      IsIncludedInLoFiEnabledFieldTrial()) {
-    lofi_status_ = IsNetworkQualityProhibitivelySlow(network_quality_estimator)
-                       ? LOFI_STATUS_ACTIVE
-                       : LOFI_STATUS_INACTIVE;
-    RecordAutoLoFiRequestHeaderStateChange(
-        connection_type_, ShouldUseLoFiHeaderForRequests(previous_lofi_status),
-        ShouldUseLoFiHeaderForRequests(lofi_status_));
-    return;
-  }
-
-  if (IsIncludedInLoFiControlFieldTrial()) {
-    lofi_status_ = IsNetworkQualityProhibitivelySlow(network_quality_estimator)
-                       ? LOFI_STATUS_ACTIVE_CONTROL
-                       : LOFI_STATUS_INACTIVE_CONTROL;
-    return;
+      IsIncludedInLoFiEnabledFieldTrial() ||
+      IsIncludedInLoFiControlFieldTrial()) {

[tbansal1, 2015/09/28 17:49:02]

So, this returns true even if user is in control group.

[megjablon, 2015/09/28 22:43:37]

Yes, we keep track of is_lofi in the frame for both the control and enabled
group now and simply don't add the header in DRPRequestParams now for the
control group.

+    return IsNetworkQualityProhibitivelySlow(network_quality_estimator);
   }
 
   // If Lo-Fi is not enabled through command line and the user is not in
-  // Lo-Fi field trials, we set Lo-Fi to permanent off.
-  lofi_status_ = LOFI_STATUS_OFF;
+  // Lo-Fi field trials, set Lo-Fi to off.
+  lofi_off_ = true;
+  return false;
 }
 
 void DataReductionProxyConfig::GetNetworkList(