Add holdback for using brotli with data saver proxies

Bug: 891458
Change-Id: I261651abb0ecfec9b46fa1a6ee8b269963de9f7e
Reviewed-on: https://chromium-review.googlesource.com/c/1258049Reviewed-by: Scott Little <sclittle@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595972}

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc

@@ -22,6 +22,7 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_util.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/core/common/lofi_decider.h"
@@ -684,6 +685,11 @@ void DataReductionProxyNetworkDelegate::MaybeAddBrotliToAcceptEncodingHeader(
     const net::URLRequest& request) const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (base::FeatureList::IsEnabled(
+          features::kDataReductionProxyBrotliHoldback)) {
+    return;
+  }
+
   // This method should be called only when the resolved proxy was a data
   // saver proxy.
   DCHECK(data_reduction_proxy_config_->FindConfiguredDataReductionProxy(

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc

@@ -1711,6 +1711,32 @@ TEST_F(DataReductionProxyNetworkDelegateTest,
   FetchURLRequestAndVerifyBrotli(nullptr, response_headers, false, false);
 }
 
+// Test that Brotli is not added to the accept-encoding header when
+// kDataReductionProxyBrotliHoldback feature is enabled.
+TEST_F(DataReductionProxyNetworkDelegateTest,
+       BrotliAdvertisement_BrotliHoldbackEnabled) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      data_reduction_proxy::features::kDataReductionProxyBrotliHoldback);
+
+  Init(USE_SECURE_PROXY, true /* enable_brotli_globally */);
+
+  ReadBrotliFile();
+
+  std::string response_headers =
+      "HTTP/1.1 200 OK\r\n"
+      "Content-Length: 140\r\n"
+      "Via: 1.1 Chrome-Compression-Proxy\r\n"
+      "Chrome-Proxy: ofcl=200\r\n"
+      "Cache-Control: max-age=1200\r\n"
+      "Vary: accept-encoding\r\n";
+  response_headers += "\r\n";
+
+  // Use secure sockets when fetching the request since Brotli is only enabled
+  // for secure connections.
+  FetchURLRequestAndVerifyBrotli(nullptr, response_headers, false, false);
+}
+
 // Test that Brotli is not added to the accept-encoding header when the request
 // is fetched from an insecure proxy.
 TEST_F(DataReductionProxyNetworkDelegateTest,

components/data_reduction_proxy/core/common/data_reduction_proxy_features.cc

@@ -50,5 +50,11 @@ const base::Feature kDataSaverSiteBreakdownUsingPageLoadMetrics{
     "DataSaverSiteBreakdownUsingPageLoadMetrics",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// If enabled, "br" is not added to the accept-encoding header. This effectively
+// disables the use of Brotli on the connection from Chrome to secure
+// HTTPS data saver proxies.
+const base::Feature kDataReductionProxyBrotliHoldback{
+    "DataReductionProxyBrotliHoldback", base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace features
 }  // namespace data_reduction_proxy

components/data_reduction_proxy/core/common/data_reduction_proxy_features.h

@@ -17,6 +17,7 @@ extern const base::Feature kMissingViaHeaderShortDuration;
 extern const base::Feature kDataReductionProxyRobustConnection;
 extern const base::Feature kDogfood;
 extern const base::Feature kDataSaverSiteBreakdownUsingPageLoadMetrics;
+extern const base::Feature kDataReductionProxyBrotliHoldback;
 
 }  // namespace features
 }  // namespace data_reduction_proxy