DataReductionProxy: Change the default warmup url.

Also, change the expected HTTP response code from 204 to a
list containing 3 codes (200, 404,and a 3rd one
configurable via field trial experiment).

Change-Id: I403841d93b9f7c8f9096e827ac67116658b15c82
Bug: 839628
Reviewed-on: https://chromium-review.googlesource.com/1043269
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556159}

components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc

@@ -187,7 +187,8 @@ void WarmupURLFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
 
   bool success_response =
       source->GetStatus().status() == net::URLRequestStatus::SUCCESS &&
-      source->GetResponseCode() == net::HTTP_NO_CONTENT &&
+      params::IsWhitelistedHttpResponseCodeForProbes(
+          source->GetResponseCode()) &&
       source->GetResponseHeaders() &&
       HasDataReductionProxyViaHeader(*(source->GetResponseHeaders()),
                                      nullptr /* has_intermediary */);

components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

@@ -231,7 +231,7 @@ TEST(WarmupURLFetcherTest, TestSuccessfulFetchWarmupURLWithViaHeader) {
   net::MockClientSocketFactory mock_socket_factory;
   net::MockRead success_reads[3];
   success_reads[0] = net::MockRead(
-      "HTTP/1.1 204 OK\r\nVia: 1.1 Chrome-Compression-Proxy\r\n\r\n");
+      "HTTP/1.1 404 NOT FOUND\r\nVia: 1.1 Chrome-Compression-Proxy\r\n\r\n");
   success_reads[1] = net::MockRead(net::ASYNC, config.c_str(), config.length());
   success_reads[2] = net::MockRead(net::SYNCHRONOUS, net::OK);
 
@@ -266,7 +266,7 @@ TEST(WarmupURLFetcherTest, TestSuccessfulFetchWarmupURLWithViaHeader) {
   histogram_tester.ExpectUniqueSample("DataReductionProxy.WarmupURL.NetError",
                                       net::OK, 1);
   histogram_tester.ExpectUniqueSample(
-      "DataReductionProxy.WarmupURL.HttpResponseCode", net::HTTP_NO_CONTENT, 1);
+      "DataReductionProxy.WarmupURL.HttpResponseCode", net::HTTP_NOT_FOUND, 1);
   histogram_tester.ExpectUniqueSample(
       "DataReductionProxy.WarmupURL.HasViaHeader", 1, 1);
   histogram_tester.ExpectUniqueSample(
@@ -468,7 +468,7 @@ TEST(WarmupURLFetcherTest, TestSuccessfulFetchWarmupURLWithDelay) {
   net::MockClientSocketFactory mock_socket_factory;
   net::MockRead success_reads[3];
   success_reads[0] = net::MockRead(
-      "HTTP/1.1 204 OK\r\nVia: 1.1 Chrome-Compression-Proxy\r\n\r\n");
+      "HTTP/1.1 404\r\nVia: 1.1 Chrome-Compression-Proxy\r\n\r\n");
   success_reads[1] = net::MockRead(net::ASYNC, config.c_str(), config.length());
   success_reads[2] = net::MockRead(net::SYNCHRONOUS, net::OK);
 
@@ -505,7 +505,7 @@ TEST(WarmupURLFetcherTest, TestSuccessfulFetchWarmupURLWithDelay) {
   histogram_tester.ExpectUniqueSample("DataReductionProxy.WarmupURL.NetError",
                                       net::OK, 1);
   histogram_tester.ExpectUniqueSample(
-      "DataReductionProxy.WarmupURL.HttpResponseCode", net::HTTP_NO_CONTENT, 1);
+      "DataReductionProxy.WarmupURL.HttpResponseCode", net::HTTP_NOT_FOUND, 1);
   histogram_tester.ExpectUniqueSample(
       "DataReductionProxy.WarmupURL.HasViaHeader", 1, 1);
   histogram_tester.ExpectUniqueSample(

components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc

@@ -19,6 +19,7 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
 #include "components/variations/variations_associated_data.h"
 #include "net/base/proxy_server.h"
+#include "net/http/http_status_code.h"
 #include "url/url_constants.h"
 
 #if defined(OS_ANDROID)
@@ -31,7 +32,7 @@ const char kEnabled[] = "Enabled";
 const char kControl[] = "Control";
 const char kDisabled[] = "Disabled";
 const char kDefaultSecureProxyCheckUrl[] = "http://check.googlezip.net/connect";
-const char kDefaultWarmupUrl[] = "http://check.googlezip.net/generate_204";
+const char kDefaultWarmupUrl[] = "http://check.googlezip.net/e2e_probe";
 
 const char kQuicFieldTrial[] = "DataReductionProxyUseQuic";
 
@@ -146,6 +147,25 @@ GURL GetWarmupURL() {
       params, "warmup_url", kDefaultWarmupUrl));
 }
 
+bool IsWhitelistedHttpResponseCodeForProbes(int http_response_code) {
+  // 200 and 404 are always whitelisted.
+  if (http_response_code == net::HTTP_OK ||
+      http_response_code == net::HTTP_NOT_FOUND) {
+    return true;
+  }
+
+  // Check if there is an additional whitelisted HTTP response code provided via
+  // the field trial params.
+  std::map<std::string, std::string> params;
+  variations::GetVariationParams(GetQuicFieldTrialName(), &params);
+  const std::string value = GetStringValueForVariationParamWithDefaultValue(
+      params, "whitelisted_probe_http_response_code", "");
+  int response_code;
+  if (!base::StringToInt(value, &response_code))
+    return false;
+  return response_code == http_response_code;
+}
+
 bool ShouldBypassMissingViaHeader(bool connection_is_cellular) {
   return GetFieldTrialParamByFeatureAsBool(
       data_reduction_proxy::features::kMissingViaHeaderShortDuration,

components/data_reduction_proxy/core/common/data_reduction_proxy_params.h

@@ -133,6 +133,12 @@ bool FetchWarmupProbeURLEnabled();
 // Returns the warmup URL.
 GURL GetWarmupURL();
 
+// Returns true if the |http_response_code| is in the whitelist of HTTP response
+// codes that are considered as successful for fetching the warmup probe URL.
+// If this method returns false, then the probe should be considered as
+// unsuccessful.
+bool IsWhitelistedHttpResponseCodeForProbes(int http_response_code);
+
 // Returns the experiment parameter name to enable the warmup fetch callback.
 const char* GetWarmupCallbackParamName();
 

components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc

@@ -21,6 +21,7 @@
 #include "components/data_reduction_proxy/proto/client_config.pb.h"
 #include "components/variations/variations_associated_data.h"
 #include "net/base/proxy_server.h"
+#include "net/http/http_status_code.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -227,8 +228,10 @@ TEST_F(DataReductionProxyParamsTest, QuicFieldTrial) {
     if (!test.enable_warmup_url)
       variation_params["enable_warmup"] = "false";
 
-    if (!test.warmup_url.empty())
+    if (!test.warmup_url.empty()) {
       variation_params["warmup_url"] = test.warmup_url;
+      variation_params["whitelisted_probe_http_response_code"] = "204";
+    }
     ASSERT_TRUE(variations::AssociateVariationParams(
         params::GetQuicFieldTrialName(), test.trial_group_name,
         variation_params));
@@ -240,14 +243,47 @@ TEST_F(DataReductionProxyParamsTest, QuicFieldTrial) {
     EXPECT_EQ(test.expected_enabled, params::IsIncludedInQuicFieldTrial());
     if (!test.warmup_url.empty()) {
       EXPECT_EQ(GURL(test.warmup_url), params::GetWarmupURL());
+      EXPECT_TRUE(params::IsWhitelistedHttpResponseCodeForProbes(200));
+      EXPECT_TRUE(params::IsWhitelistedHttpResponseCodeForProbes(net::HTTP_OK));
+      EXPECT_TRUE(params::IsWhitelistedHttpResponseCodeForProbes(204));
+      EXPECT_FALSE(params::IsWhitelistedHttpResponseCodeForProbes(302));
+      EXPECT_FALSE(params::IsWhitelistedHttpResponseCodeForProbes(307));
+      EXPECT_TRUE(params::IsWhitelistedHttpResponseCodeForProbes(404));
     } else {
-      EXPECT_EQ(GURL("http://check.googlezip.net/generate_204"),
+      EXPECT_EQ(GURL("http://check.googlezip.net/e2e_probe"),
                 params::GetWarmupURL());
     }
     EXPECT_TRUE(params::FetchWarmupProbeURLEnabled());
   }
 }
 
+TEST_F(DataReductionProxyParamsTest, QuicFieldTrialDefaultResponseCodeWarmup) {
+  ASSERT_FALSE(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kDisableDataReductionProxyWarmupURLFetch));
+
+  EXPECT_TRUE(params::IsIncludedInQuicFieldTrial());
+  EXPECT_EQ(GURL("http://check.googlezip.net/e2e_probe"),
+            params::GetWarmupURL());
+  EXPECT_TRUE(params::FetchWarmupProbeURLEnabled());
+
+  const struct {
+    int http_response_code;
+    bool expected_whitelisted;
+  } tests[] = {{200, true},
+               {net::HTTP_OK, true},
+               {204, false},
+               {301, false},
+               {net::HTTP_TEMPORARY_REDIRECT, false},
+               {404, true},
+               {net::HTTP_NOT_FOUND, true}};
+
+  for (const auto& test : tests) {
+    EXPECT_EQ(test.expected_whitelisted,
+              params::IsWhitelistedHttpResponseCodeForProbes(
+                  test.http_response_code));
+  }
+}
+
 // Tests if the QUIC field trial |enable_quic_non_core_proxies| is set
 // correctly.
 TEST_F(DataReductionProxyParamsTest, QuicEnableNonCoreProxies) {