New CPAT support in DataReductionProxyConfig guarded by feature flag.

components/data_reduction_proxy/core/browser/data_reduction_proxy_config.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 "components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/macros.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/default_tick_clock.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_config_values.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/previews/core/previews_decider.h"
-#include "components/previews/core/previews_experiments.h"
 #include "components/variations/variations_associated_data.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/load_flags.h"
 #include "net/base/network_change_notifier.h"
 #include "net/log/net_log_source_type.h"
 #include "net/nqe/effective_connection_type.h"
 #include "net/proxy/proxy_server.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
@@ skipping 953 matching lines @@
 }
 
 bool DataReductionProxyConfig::IsEffectiveConnectionTypeSlowerThanThreshold(
     net::EffectiveConnectionType effective_connection_type) const {
   return effective_connection_type >= net::EFFECTIVE_CONNECTION_TYPE_OFFLINE &&
          effective_connection_type <= lofi_effective_connection_type_threshold_;
 }
 
 bool DataReductionProxyConfig::ShouldEnableLoFi(
     const net::URLRequest& request,
-    previews::PreviewsDecider* previews_decider) {
-  DCHECK(previews_decider);
+    const previews::PreviewsDecider& previews_decider) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK((request.load_flags() & net::LOAD_MAIN_FRAME_DEPRECATED) != 0);
   DCHECK(!request.url().SchemeIsCryptographic());
 
+  if (base::FeatureList::IsEnabled(
+          features::kDataReductionProxyDecidesTransform))

[megjablon, 2017/05/22 23:58:57]

Can this check just be moved inside the Internal methods around the network
delegate code so we don't end up with so much duplicate code?

[dougarnett, 2017/05/23 16:28:44]

My intention is to delete the *Internal methods in cleanup pass after stable
(would then be easy to review change). Also was thinking of dropping this method
as well and just going with the new ShouldAccept* naming to be more in line with
it just driving populating CPAT header without so much extra logic. This would
include removing this feature definition and so the feature check as well.

+    return ShouldAcceptServerLoFi(request, previews_decider);
+
   bool enable_lofi = ShouldEnableLoFiInternal(request, previews_decider);
 
   if (params::IsLoFiSlowConnectionsOnlyViaFlags() ||
       params::IsIncludedInLoFiEnabledFieldTrial()) {
     RecordAutoLoFiRequestHeaderStateChange(
         connection_type_, previous_state_lofi_on_, enable_lofi);
     previous_state_lofi_on_ = enable_lofi;
   }
 
   return enable_lofi;
 }
 
 bool DataReductionProxyConfig::ShouldEnableLitePages(
     const net::URLRequest& request,
-    previews::PreviewsDecider* previews_decider) {
-  DCHECK(previews_decider);
+    const previews::PreviewsDecider& previews_decider) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK((request.load_flags() & net::LOAD_MAIN_FRAME_DEPRECATED) != 0);
   DCHECK(!request.url().SchemeIsCryptographic());
 
+  if (base::FeatureList::IsEnabled(
+          features::kDataReductionProxyDecidesTransform))

[megjablon, 2017/05/22 23:58:57]

Same here

[dougarnett, 2017/05/23 16:28:45]

Acknowledged.

+    return ShouldAcceptLitePages(request, previews_decider);
+
   return ShouldEnableLitePagesInternal(request, previews_decider);
 }
 
 bool DataReductionProxyConfig::enabled_by_user_and_reachable() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return enabled_by_user_ && !unreachable_;
 }
 
+bool DataReductionProxyConfig::IsBlackListedOrDisabled(
+    const net::URLRequest& request,
+    const previews::PreviewsDecider& previews_decider,
+    previews::PreviewsType previews_type) {
+  // Make sure request is not locally blacklisted.
+  if (params::IsBlackListEnabledForServerPreviews()) {
+    // Pass in net::EFFECTIVE_CONNECTION_TYPE_4G as the thresold as network
+    // speed is checked in IsNetworkQualityProhibitivelySlow().
+    // TODO(ryansturm): Use the correct ECT value (or add new method to
+    // just check blacklist). crbug.com/720102
+    return !previews_decider.ShouldAllowPreviewAtECT(
+        request, previews_type, net::EFFECTIVE_CONNECTION_TYPE_4G);
+  } else {
+    // If Lo-Fi has been turned off, its status can't change. This Lo-Fi bit
+    // will be removed when Lo-Fi and Lite Pages are moved over to using the
+    // PreviewsBlackList.
+    return lofi_off_;
+  }
+}
+
+bool DataReductionProxyConfig::ShouldAcceptServerLoFi(
+    const net::URLRequest& request,
+    const previews::PreviewsDecider& previews_decider) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(base::FeatureList::IsEnabled(
+      features::kDataReductionProxyDecidesTransform));
+
+  // LitePages overrides Server Lo-Fi. Server will direct Server Lo-Fi for
+  // resources if applicable.
+  if (ShouldEnableLitePages(request, previews_decider)) {
+    return false;
+  }
+
+  if (IsBlackListedOrDisabled(request, previews_decider,
+                              previews::PreviewsType::LOFI))
+    return false;
+
+  if (params::IsLoFiAlwaysOnViaFlags())
+    return true;
+
+  if (params::IsLoFiCellularOnlyViaFlags()) {
+    return net::NetworkChangeNotifier::IsConnectionCellular(connection_type_);
+  }
+
+  if (params::IsLoFiSlowConnectionsOnlyViaFlags() ||
+      params::IsIncludedInLoFiEnabledFieldTrial()) {
+    // Accept Lo-Fi from the data reduction proxy (it will handle the effective
+    // connection type check).
+    return true;
+  }
+
+  return false;
+}
+
+bool DataReductionProxyConfig::ShouldAcceptLitePages(
+    const net::URLRequest& request,
+    const previews::PreviewsDecider& previews_decider) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(base::FeatureList::IsEnabled(
+      features::kDataReductionProxyDecidesTransform));
+
+  if (IsBlackListedOrDisabled(request, previews_decider,
+                              previews::PreviewsType::LITE_PAGE))
+    return false;
+
+  if (params::IsLoFiAlwaysOnViaFlags() && params::AreLitePagesEnabledViaFlags())
+    return true;
+
+  if (params::IsLoFiCellularOnlyViaFlags() &&
+      params::AreLitePagesEnabledViaFlags()) {
+    return net::NetworkChangeNotifier::IsConnectionCellular(connection_type_);
+  }
+
+  if ((params::IsLoFiSlowConnectionsOnlyViaFlags() &&
+       params::AreLitePagesEnabledViaFlags()) ||
+      params::IsIncludedInLitePageFieldTrial()) {
+    // Accept LitePages from the data reduction proxy (it will handle the
+    // effective connection type check).
+    return true;
+  }
+
+  return false;
+}
+
 bool DataReductionProxyConfig::ShouldEnableLoFiInternal(
     const net::URLRequest& request,
-    previews::PreviewsDecider* previews_decider) {
+    const previews::PreviewsDecider& previews_decider) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!base::FeatureList::IsEnabled(
+      features::kDataReductionProxyDecidesTransform));
 
   last_query_ = GetTicksNow();
   network_quality_at_last_query_ = NETWORK_QUALITY_AT_LAST_QUERY_UNKNOWN;
 
-  if (params::IsBlackListEnabledForServerPreviews()) {
-    // Pass in net::EFFECTIVE_CONNECTION_TYPE_4G as the thresold as network
-    // speed is checked in IsNetworkQualityProhibitivelySlow().
-    // TODO(ryansturm): Use the correct ECT value. crbug.com/720102
-    if (!previews_decider->ShouldAllowPreviewAtECT(
-            request, previews::PreviewsType::LOFI,
-            net::EFFECTIVE_CONNECTION_TYPE_4G)) {
-      return false;
-    }
-  } else if (lofi_off_) {
-    // If Lo-Fi has been turned off, its status can't change. This Lo-Fi bit
-    // will be removed when Lo-Fi and Lite Pages are moved over to using the
-    // PreviewsBlackList.
+  if (IsBlackListedOrDisabled(request, previews_decider,
+                              previews::PreviewsType::LOFI))

[megjablon, 2017/05/22 23:58:57]

Use braces

[dougarnett, 2017/05/23 16:28:44]

Done.

     return false;
-  }
 
   if (params::IsLoFiAlwaysOnViaFlags())
     return true;
 
   if (params::IsLoFiCellularOnlyViaFlags()) {
     return net::NetworkChangeNotifier::IsConnectionCellular(connection_type_);
   }
 
   net::NetworkQualityEstimator* network_quality_estimator;
   network_quality_estimator =
       request.context() ? request.context()->network_quality_estimator()
                         : nullptr;
 
   if (params::IsLoFiSlowConnectionsOnlyViaFlags() ||
       params::IsIncludedInLoFiEnabledFieldTrial() ||
       params::IsIncludedInLoFiControlFieldTrial()) {
     return IsNetworkQualityProhibitivelySlow(network_quality_estimator);
   }
 
   return false;
 }
 
 bool DataReductionProxyConfig::ShouldEnableLitePagesInternal(
     const net::URLRequest& request,
-    previews::PreviewsDecider* previews_decider) {
+    const previews::PreviewsDecider& previews_decider) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!base::FeatureList::IsEnabled(
+      features::kDataReductionProxyDecidesTransform));
 
-  if (params::IsBlackListEnabledForServerPreviews()) {
-    // Pass in net::EFFECTIVE_CONNECTION_TYPE_4G as the thresold as network
-    // speed is checked in IsNetworkQualityProhibitivelySlow().
-    // TODO(ryansturm): Use the correct ECT value. crbug.com/720102
-    if (!previews_decider->ShouldAllowPreviewAtECT(
-            request, previews::PreviewsType::LITE_PAGE,
-            net::EFFECTIVE_CONNECTION_TYPE_4G)) {
-      return false;
-    }
-  } else if (lofi_off_) {
-    // If Lo-Fi has been turned off, its status can't change. This Lo-Fi bit
-    // will be removed when Lo-Fi and Lite Pages are moved over to using the
-    // PreviewsBlackList.
+  if (IsBlackListedOrDisabled(request, previews_decider,
+                              previews::PreviewsType::LITE_PAGE))

[megjablon, 2017/05/22 23:58:57]

Use braces

[dougarnett, 2017/05/23 16:28:45]

Done.

     return false;
-  }
 
   if (params::IsLoFiAlwaysOnViaFlags() && params::AreLitePagesEnabledViaFlags())
     return true;
 
   if (params::IsLoFiCellularOnlyViaFlags() &&
       params::AreLitePagesEnabledViaFlags()) {
     return net::NetworkChangeNotifier::IsConnectionCellular(
         net::NetworkChangeNotifier::GetConnectionType());
   }
   net::NetworkQualityEstimator* network_quality_estimator;
@@ skipping 44 matching lines @@
 DataReductionProxyConfig::GetProxiesForHttp() const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!enabled_by_user_)
     return std::vector<DataReductionProxyServer>();
 
   return config_values_->proxies_for_http();
 }
 
 }  // namespace data_reduction_proxy