Add experiment to not bypass missing via header

Adds an experimental param on the connection robustness mechanism to
disable proxy bypass when the via header is missing.

Bug: 724704
Change-Id: I56286a06485b70c0c746850de14d79b2da755366
Reviewed-on: https://chromium-review.googlesource.com/990775
Commit-Queue: Robert Ogden <robertogden@google.com>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547793}

components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc

@@ -171,7 +171,7 @@ void WarmupURLFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
 
   if (!GetFieldTrialParamByFeatureAsBool(
           features::kDataReductionProxyRobustConnection,
-          "warmup_fetch_callback_enabled", false)) {
+          params::GetWarmupCallbackParamName(), false)) {
     CleanupAfterFetch();
     return;
   }
@@ -257,7 +257,7 @@ void WarmupURLFetcher::OnFetchTimeout() {
 
   if (!GetFieldTrialParamByFeatureAsBool(
           features::kDataReductionProxyRobustConnection,
-          "warmup_fetch_callback_enabled", false)) {
+          params::GetWarmupCallbackParamName(), false)) {
     // Running the callback is not enabled.
     CleanupAfterFetch();
     return;

components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

@@ -18,6 +18,7 @@
 #include "base/threading/platform_thread.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_params.h"
 #include "net/base/proxy_server.h"
 #include "net/http/http_status_code.h"
 #include "net/nqe/network_quality_estimator_test_util.h"
@@ -52,7 +53,7 @@ class WarmupURLFetcherTest : public WarmupURLFetcher {
   static void InitExperiment(
       base::test::ScopedFeatureList* scoped_feature_list) {
     std::map<std::string, std::string> params;
-    params["warmup_fetch_callback_enabled"] = "true";
+    params[params::GetWarmupCallbackParamName()] = "true";
     scoped_feature_list->InitAndEnableFeatureWithParameters(
         features::kDataReductionProxyRobustConnection, params);
   }

components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc

@@ -11,12 +11,14 @@
 #include <utility>
 #include <vector>
 
+#include "base/metrics/field_trial_params.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.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 "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
@@ -423,7 +425,17 @@ DataReductionProxyBypassType GetDataReductionProxyBypassType(
       !headers.HasHeader("Proxy-Authenticate")) {
     return BYPASS_EVENT_TYPE_MALFORMED_407;
   }
-  if (!has_via_header && (headers.response_code() != net::HTTP_NOT_MODIFIED)) {
+
+  bool disable_bypass_on_missing_via_header =
+      GetFieldTrialParamByFeatureAsBool(
+          features::kDataReductionProxyRobustConnection,
+          params::GetWarmupCallbackParamName(), false) &&
+      GetFieldTrialParamByFeatureAsBool(
+          features::kDataReductionProxyRobustConnection,
+          params::GetMissingViaBypassParamName(), false);
+
+  if (!has_via_header && !disable_bypass_on_missing_via_header &&
+      (headers.response_code() != net::HTTP_NOT_MODIFIED)) {
     // A Via header might not be present in a 304. Since the goal of a 304
     // response is to minimize information transfer, a sender in general
     // should not generate representation metadata other than Cache-Control,

components/data_reduction_proxy/core/common/data_reduction_proxy_headers_unittest.cc

@@ -18,6 +18,7 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_storage_delegate_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers_test_utils.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "net/http/http_response_headers.h"
 #include "net/proxy_resolution/proxy_resolution_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -665,6 +666,56 @@ TEST_F(DataReductionProxyHeadersTest, MissingViaHeaderFallback) {
   }
 }
 
+TEST_F(DataReductionProxyHeadersTest, BypassMissingViaIfExperiment) {
+  const struct {
+    const char* headers;
+    std::map<std::string, std::string> feature_parameters;
+    DataReductionProxyBypassType expected_result;
+  } tests[] = {
+      {
+          "HTTP/1.1 200 OK\n",
+          {
+              {params::GetWarmupCallbackParamName(), "true"},
+              {params::GetMissingViaBypassParamName(), "true"},
+          },
+          BYPASS_EVENT_TYPE_MAX,
+      },
+      {
+          "HTTP/1.1 200 OK\n",
+          {
+              {params::GetWarmupCallbackParamName(), "true"},
+              {params::GetMissingViaBypassParamName(), "false"},
+          },
+          BYPASS_EVENT_TYPE_MISSING_VIA_HEADER_OTHER,
+      },
+      {
+          "HTTP/1.1 200 OK\n",
+          {
+              {params::GetWarmupCallbackParamName(), "false"},
+              {params::GetMissingViaBypassParamName(), "false"},
+          },
+          BYPASS_EVENT_TYPE_MISSING_VIA_HEADER_OTHER,
+      },
+  };
+  for (auto test : tests) {
+    std::string headers(test.headers);
+    HeadersToRaw(&headers);
+    scoped_refptr<net::HttpResponseHeaders> parsed(
+        new net::HttpResponseHeaders(headers));
+    DataReductionProxyInfo proxy_info;
+
+    base::test::ScopedFeatureList scoped_feature_list;
+    scoped_feature_list.InitAndEnableFeatureWithParameters(
+        features::kDataReductionProxyRobustConnection, test.feature_parameters);
+
+    EXPECT_EQ(test.expected_result,
+              GetDataReductionProxyBypassType(std::vector<GURL>(), *parsed,
+                                              &proxy_info));
+    if (test.expected_result != BYPASS_EVENT_TYPE_MAX)
+      EXPECT_TRUE(proxy_info.mark_proxies_as_bad);
+  }
+}
+
 TEST_F(DataReductionProxyHeadersTest, GetDataReductionProxyBypassEventType) {
   const struct {
      const char* headers;

components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc

@@ -53,6 +53,9 @@ const char kServerExperimentsFieldTrial[] =
 // LitePage black list version.
 const char kLitePageBlackListVersion[] = "lite-page-blacklist-version";
 
+const char kWarmupFetchCallbackEnabledParam[] = "warmup_fetch_callback_enabled";
+const char kMissingViaBypassDisabledParam[] = "bypass_missing_via_disabled";
+
 bool IsIncludedInFieldTrial(const std::string& name) {
   return base::StartsWith(base::FieldTrialList::FindFullName(name), kEnabled,
                           base::CompareCase::SENSITIVE);
@@ -115,6 +118,14 @@ const char* GetLoFiFlagFieldTrialName() {
   return kLoFiFlagFieldTrial;
 }
 
+const char* GetWarmupCallbackParamName() {
+  return kWarmupFetchCallbackEnabledParam;
+}
+
+const char* GetMissingViaBypassParamName() {
+  return kMissingViaBypassDisabledParam;
+}
+
 bool IsIncludedInServerExperimentsFieldTrial() {
   return !base::CommandLine::ForCurrentProcess()->HasSwitch(
              data_reduction_proxy::switches::

components/data_reduction_proxy/core/common/data_reduction_proxy_params.h

@@ -133,6 +133,12 @@ bool FetchWarmupProbeURLEnabled();
 // Returns the warmup URL.
 GURL GetWarmupURL();
 
+// Returns the experiment parameter name to enable the warmup fetch callback.
+const char* GetWarmupCallbackParamName();
+
+// Returns the experiment parameter name to disable missing via header bypasses.
+const char* GetMissingViaBypassParamName();
+
 }  // namespace params
 
 // Contains information about a given proxy server. |proxies_for_http| contains