Record errors that trigger a data reduction proxy bypass

net/http/http_network_layer_unittest.cc

Index: net/http/http_network_layer_unittest.cc
diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc
index e0ecb53604a942c72c45347896511cc73128f2bb..4d1e718eb7eb090df6e7f4d1009c5388abe61d5c 100644
--- a/net/http/http_network_layer_unittest.cc
+++ b/net/http/http_network_layer_unittest.cc
@@ -476,31 +476,56 @@ TEST_F(HttpNetworkLayerTest, ServerOneProxyNoDirectBypassFixed) {
   TestProxyFallbackFail(1u, bad_proxy, "");
 }
 
-TEST_F(HttpNetworkLayerTest, ServerFallbackOn5xxError) {
+TEST_F(HttpNetworkLayerTest, ServerFallbackOn4xxAnd5xxErrors) {
   // Verify that "500 Internal Server Error", "502 Bad Gateway", and
   // "503 Service Unavailable" via the data reduction proxy induce proxy
   // fallback to a second proxy, if configured.
 
+  // Verify that 4xx responses bypass both data reduction proxies, when
+  // the proxy originates them.
+
   // To configure this test, we need to wire up a custom proxy service to use
-  // a pair of proxies. We'll induce fallback via the first and return
-  // the expected data via the second.
+  // a pair or triplet of proxies. We'll induce fallback via the first and
+  // return the expected data via the second, or third if the first two are
+  // both bypassed.
   std::string data_reduction_proxy(
       HostPortPair::FromURL(GURL(SPDY_PROXY_AUTH_ORIGIN)).ToString());
-  std::string pac_string = base::StringPrintf(
-      "PROXY %s; PROXY good:8080", data_reduction_proxy.data());
+  std::string data_reduction_proxy_fallback;
+  size_t data_reduction_proxy_count = 1u;
+#if defined(DATA_REDUCTION_FALLBACK_HOST)
+  data_reduction_proxy_fallback =
+      HostPortPair::FromURL(GURL(DATA_REDUCTION_FALLBACK_HOST)).ToString();
+  data_reduction_proxy_count = 2u;
+#endif
 
-  std::string headers[] = {
-    "HTTP/1.1 500 Internal Server Error\r\n\r\n",
-    "HTTP/1.1 502 Bad Gateway\r\n\r\n",
-    "HTTP/1.1 503 Service Unavailable\r\n\r\n"
+
+  const struct {
+    const char* headers;
+    size_t expected_proxies_on_retry_list;
+  } tests[] = {
+      { "HTTP/1.1 500 Internal Server Error\r\n\r\n", 1u },
+      { "HTTP/1.1 502 Bad Gateway\r\n\r\n", 1u },
+      { "HTTP/1.1 503 Service Unavailable\r\n\r\n", 1u },
+      { "HTTP/1.1 414 Request-URI Too Long\r\nServer: GFE/2.0\r\n\r\n",
+        data_reduction_proxy_count },
+      { "HTTP/1.1 414 Request-URI Too Long\r\n\r\n", 1u}
   };
 
-  for (size_t i = 0; i < arraysize(headers); ++i) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+    std::string pac_string;
+    if (data_reduction_proxy_count == 1u) {
+      pac_string = base::StringPrintf(
+          "PROXY %s; PROXY good:8080", data_reduction_proxy.data());
+    } else {
+      pac_string = base::StringPrintf(
+          "PROXY %s; PROXY %s; PROXY good:8080", data_reduction_proxy.data(),
+          data_reduction_proxy_fallback.data());
+    }
     ConfigureTestDependencies(
         ProxyService::CreateFixedFromPacResult(pac_string));
 
     MockRead data_reads[] = {
-      MockRead(headers[i].c_str()),
+      MockRead(tests[i].headers),
       MockRead("Bypass message"),
       MockRead(SYNCHRONOUS, OK),
     };
@@ -518,7 +543,8 @@ TEST_F(HttpNetworkLayerTest, ServerFallbackOn5xxError) {
     // Second data provider returns the expected content.
     MockRead data_reads2[] = {
       MockRead("HTTP/1.0 200 OK\r\n"
-               "Server: not-proxy\r\n\r\n"),
+               "Server: good-proxy\r\n"

[mef, 2014/05/29 16:41:38]

Is this the data that is expected from server? Why did it change to
'good-proxy'?

[bengr, 2014/05/29 18:46:19]

"Not proxy" was incorrect. The test falls from a bad proxy to a good one.

+               "Via: 1.1 Chrome-Compression-Proxy\r\n\r\n"),
       MockRead("content"),
       MockRead(SYNCHRONOUS, OK),
     };
@@ -557,11 +583,19 @@ TEST_F(HttpNetworkLayerTest, ServerFallbackOn5xxError) {
     EXPECT_EQ("content", contents);
     // We also have a server header here that isn't set by the proxy.
     EXPECT_TRUE(trans->GetResponseInfo()->headers->HasHeaderValue(
-        "server", "not-proxy"));
+        "server", "good-proxy"));
     // We should also observe the data reduction proxy in the retry list.
-    ASSERT_EQ(1u, proxy_service_->proxy_retry_info().size());
-    EXPECT_EQ(data_reduction_proxy,
-              (*proxy_service_->proxy_retry_info().begin()).first);
+    ASSERT_EQ(tests[i].expected_proxies_on_retry_list,
+              proxy_service_->proxy_retry_info().size());
+    if (tests[i].expected_proxies_on_retry_list == 1u) {
+      EXPECT_EQ(data_reduction_proxy,
+                (*proxy_service_->proxy_retry_info().begin()).first);
+    } else {
+      ProxyRetryInfoMap::const_iterator i =

[mef, 2014/05/29 16:41:38]

Given that this is a map I think it is too brittle to expect particular order of
proxies. 

I'd suggest EXPECT_EQ(1u,
proxy_service_->proxy_retry_info().count(data_reduction_proxy_fallback));

[bengr, 2014/05/29 18:46:19]

Good suggestion. Thanks. Done.

+          (proxy_service_->proxy_retry_info()).begin();
+      EXPECT_EQ(data_reduction_proxy_fallback, (*i++).first);
+      EXPECT_EQ(data_reduction_proxy, (*i).first);
+    }
   }
 }
 #endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
@@ -671,6 +705,25 @@ TEST_F(HttpNetworkLayerTest, ServerFallbackWithNoViaHeader) {
                                  arraysize(data_reads), 1u);
 }
 
+TEST_F(HttpNetworkLayerTest, NoServerFallbackWith4xxFromOrigin) {
+  // Verify that Chrome will not be induced to bypass the data reduction proxy
+  // when the data reduction proxy via header is absent on a 4xx.

[mef, 2014/05/29 16:41:38]

Is this a valid comment? I see 'Via' header in mock reads...

[bengr, 2014/05/29 18:46:19]

Done.

+  std::string chrome_proxy = GetDataReductionProxy();
+  ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult(
+      "PROXY " + chrome_proxy + "; PROXY good:8080"));
+
+  MockRead data_reads[] = {
+    MockRead("HTTP/1.1 414 Request-URI Too Long\r\n"
+             "Connection: keep-alive\r\n"
+             "Via: 1.1 Chrome-Compression-Proxy\r\n\r\n"),
+    MockRead(SYNCHRONOUS, OK),
+  };
+
+  TestProxyFallbackByMethodWithMockReads(chrome_proxy, std::string(),
+                                         data_reads, arraysize(data_reads),
+                                         "GET", std::string(), false, 0);
+}
+
 TEST_F(HttpNetworkLayerTest, NoServerFallbackWith304Response) {
   // Verify that Chrome will not be induced to bypass the data reduction proxy
   // when the data reduction proxy via header is absent on a 304.