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..e0cf89037062fa1a5d3a817b25a2e300f2995108 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
@@ -77,87 +77,6 @@ bool FindProxyInList(const std::vector<net::ProxyServer>& proxy_list,
   return false;
 }
 
-// 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) {
-  // 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
-  // metrics/histograms/histograms.xml.
-  enum AutoLoFiRequestHeaderState {
-    AUTO_LOFI_REQUEST_HEADER_STATE_EMPTY_TO_EMPTY = 0,
-    AUTO_LOFI_REQUEST_HEADER_STATE_EMPTY_TO_LOW = 1,
-    AUTO_LOFI_REQUEST_HEADER_STATE_LOW_TO_EMPTY = 2,
-    AUTO_LOFI_REQUEST_HEADER_STATE_LOW_TO_LOW = 3,
-    AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY
-  };
-
-  AutoLoFiRequestHeaderState state;
-  if (!previous_header_low) {
-    if (current_header_low)
-      state = AUTO_LOFI_REQUEST_HEADER_STATE_EMPTY_TO_LOW;
-    else
-      state = AUTO_LOFI_REQUEST_HEADER_STATE_EMPTY_TO_EMPTY;
-  } else {
-    if (current_header_low) {
-      // Low to low in useful in checking how many consecutive page loads
-      // are done with Lo-Fi enabled.
-      state = AUTO_LOFI_REQUEST_HEADER_STATE_LOW_TO_LOW;
-    } else {
-      state = AUTO_LOFI_REQUEST_HEADER_STATE_LOW_TO_EMPTY;
-    }
-  }
-
-  switch (connection_type) {
-    case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.Unknown", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.Ethernet", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    case net::NetworkChangeNotifier::CONNECTION_WIFI:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.WiFi", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    case net::NetworkChangeNotifier::CONNECTION_2G:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.2G", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    case net::NetworkChangeNotifier::CONNECTION_3G:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.3G", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    case net::NetworkChangeNotifier::CONNECTION_4G:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.4G", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    case net::NetworkChangeNotifier::CONNECTION_NONE:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.None", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
-      UMA_HISTOGRAM_ENUMERATION(
-          "DataReductionProxy.AutoLoFiRequestHeaderState.Bluetooth", state,
-          AUTO_LOFI_REQUEST_HEADER_STATE_INDEX_BOUNDARY);
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
-}
-
 }  // namespace
 
 namespace data_reduction_proxy {
@@ -243,7 +162,7 @@ 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),

[bengr, 2015/08/25 00:00:01]

Why this change? Does this mean lofi is on until we learn it shouldn't be?

[megjablon, 2015/08/25 20:29:46]

lofi_off sets lofi off for the rest of the session. Once we enter the state, it
isn't reset until a new session starts. The idea temporarily off doesn't exist
anymore since that was set when lofi was being turned off due to bypassing the
cache, which we are no longer using as a signal. We bypass the cache by setting
the state to LOFI_OFF in RenderFrameImpl. This new value is simply used for
implicit opt out.

       last_main_frame_request_(base::TimeTicks::Now()),
       network_quality_at_last_main_frame_request_(
           NETWORK_QUALITY_AT_LAST_MAIN_FRAME_REQUEST_UNKNOWN) {
@@ -462,38 +381,6 @@ bool DataReductionProxyConfig::IsIncludedInLoFiControlFieldTrial() const {
          kControl;
 }
 
-LoFiStatus DataReductionProxyConfig::GetLoFiStatus() const {

[bengr, 2015/08/25 00:00:01]

More deleted code. Why?

[megjablon, 2015/08/25 20:29:46]

LoFiStatus doesn't exist anymore, we base Lo-Fi being used on the new LoFiState
and checking that we're not in the control group in DRPRequestOptions.
Therefore, none of this code is necessary.

-  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 +650,7 @@ void DataReductionProxyConfig::SecureProxyCheck(
 
 void DataReductionProxyConfig::SetLoFiModeOff() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  lofi_status_ = LOFI_STATUS_OFF;
+  lofi_off_ = true;
 }
 
 void DataReductionProxyConfig::RecordAutoLoFiAccuracyRate(
@@ -863,8 +750,7 @@ void DataReductionProxyConfig::RecordAutoLoFiAccuracyRate(
   }
 }
 
-void DataReductionProxyConfig::UpdateLoFiStatusOnMainFrameRequest(
-    bool user_temporarily_disabled_lofi,
+bool DataReductionProxyConfig::ShouldTurnOnLoFiOnMainFrameRequest(
     const net::NetworkQualityEstimator* network_quality_estimator) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
@@ -882,77 +768,34 @@ 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()) {

[bengr, 2015/08/25 00:00:01]

Remove curly braces.

[megjablon, 2015/08/25 20:29:46]

Done.

-    lofi_status_ = LOFI_STATUS_ACTIVE_FROM_FLAGS;
-    return;
+    return true;
   }
 
   if (params::IsLoFiCellularOnlyViaFlags()) {
     if (net::NetworkChangeNotifier::IsConnectionCellular(
             net::NetworkChangeNotifier::GetConnectionType())) {
-      lofi_status_ = LOFI_STATUS_ACTIVE_FROM_FLAGS;
-      return;
+      return true;
     }
-    lofi_status_ = LOFI_STATUS_TEMPORARILY_OFF;
-    return;
+    return false;
   }
 
-  // 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()) {
+    if (IsNetworkQualityProhibitivelySlow(network_quality_estimator))
+      return true;
+    return false;
   }
 
   // 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(