Revert of https://codereview.chromium.org/99283006/

Reason for revert: Broke Android bot again

TBR=mef@chromium.org,mmenke@chromium.org,bengr@chromium.org
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/107073004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241954 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 23 insertions, 118 deletions

diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc
index 6ce0bc8..174ae2e 100644
--- a/net/http/http_network_layer_unittest.cc
+++ b/net/http/http_network_layer_unittest.cc
@@ -4,7 +4,6 @@
 
 #include "net/http/http_network_layer.h"
 
-#include "base/basictypes.h"
 #include "base/strings/stringprintf.h"
 #include "net/base/net_log.h"
 #include "net/cert/mock_cert_verifier.h"
@@ -61,15 +60,14 @@ class HttpNetworkLayerTest : public PlatformTest {
   }
 #endif
 
-  void ExecuteRequestExpectingContentAndHeader(const std::string& method,
-                                               const std::string& content,
+  void ExecuteRequestExpectingContentAndHeader(const std::string& content,
                                                const std::string& header,
                                                const std::string& value) {
     TestCompletionCallback callback;
 
     HttpRequestInfo request_info;
     request_info.url = GURL("http://www.google.com/");
-    request_info.method = method;
+    request_info.method = "GET";
     request_info.load_flags = LOAD_NORMAL;
 
     scoped_ptr<HttpTransaction> trans;
@@ -124,31 +122,10 @@ class HttpNetworkLayerTest : public PlatformTest {
                                       MockRead data_reads[],
                                       int data_reads_size,
                                       unsigned int expected_retry_info_size) {
-    TestProxyFallbackByMethodWithMockReads(bad_proxy, bad_proxy2, data_reads,
-                                           data_reads_size, "GET", "content",
-                                           true, expected_retry_info_size);
-  }
-
-  void TestProxyFallbackByMethodWithMockReads(
-      const std::string& bad_proxy,
-      const std::string& bad_proxy2,
-      MockRead data_reads[],
-      int data_reads_size,
-      std::string method,
-      std::string content,
-      bool retry_expected,
-      unsigned int expected_retry_info_size) {
-    std::string trailer =
-        (method == "HEAD" || method == "PUT" || method == "POST") ?
-        "Content-Length: 0\r\n\r\n" : "\r\n";
-    std::string request =
-        base::StringPrintf("%s http://www.google.com/ HTTP/1.1\r\n"
-                           "Host: www.google.com\r\n"
-                           "Proxy-Connection: keep-alive\r\n"
-                           "%s", method.c_str(), trailer.c_str());
-
     MockWrite data_writes[] = {
-      MockWrite(request.c_str()),
+      MockWrite("GET http://www.google.com/ HTTP/1.1\r\n"
+                "Host: www.google.com\r\n"
+                "Proxy-Connection: keep-alive\r\n\r\n"),
     };
 
     StaticSocketDataProvider data1(data_reads, data_reads_size,
@@ -156,29 +133,24 @@ class HttpNetworkLayerTest : public PlatformTest {
     mock_socket_factory_.AddSocketDataProvider(&data1);
 
     // Second data provider returns the expected content.
-    MockRead data_reads2[3];
-    size_t data_reads2_index = 0;
-    data_reads2[data_reads2_index++] = MockRead("HTTP/1.0 200 OK\r\n"
-                                                "Server: not-proxy\r\n\r\n");
-    if (!content.empty())
-      data_reads2[data_reads2_index++] = MockRead(content.c_str());
-    data_reads2[data_reads2_index++] = MockRead(SYNCHRONOUS, OK);
-
+    MockRead data_reads2[] = {
+      MockRead("HTTP/1.0 200 OK\r\n"
+               "Server: not-proxy\r\n\r\n"),
+      MockRead("content"),
+      MockRead(SYNCHRONOUS, OK),
+    };
     MockWrite data_writes2[] = {
-      MockWrite(request.c_str()),
+      MockWrite("GET http://www.google.com/ HTTP/1.1\r\n"
+                "Host: www.google.com\r\n"
+                "Proxy-Connection: keep-alive\r\n\r\n"),
     };
-    StaticSocketDataProvider data2(data_reads2, data_reads2_index,
+    StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
                                   data_writes2, arraysize(data_writes2));
     mock_socket_factory_.AddSocketDataProvider(&data2);
 
     // Expect that we get "content" and not "Bypass message", and that there's
     // a "not-proxy" "Server:" header in the final response.
-    if (retry_expected) {
-      ExecuteRequestExpectingContentAndHeader(method, content,
-                                              "server", "not-proxy");
-    } else {
-      ExecuteRequestExpectingContentAndHeader(method, "Bypass message", "", "");
-    }
+    ExecuteRequestExpectingContentAndHeader("content", "server", "not-proxy");
 
     // We should also observe the bad proxy in the retry list.
     TestBadProxies(expected_retry_info_size, bad_proxy, bad_proxy2);
@@ -222,8 +194,7 @@ class HttpNetworkLayerTest : public PlatformTest {
 
     // Expect that we get "content" and not "Bypass message", and that there's
     // a "not-proxy" "Server:" header in the final response.
-    ExecuteRequestExpectingContentAndHeader("GET", "content",
-                                            "server", "not-proxy");
+    ExecuteRequestExpectingContentAndHeader("content", "server", "not-proxy");
 
     // We should also observe the bad proxy in the retry list.
     TestBadProxies(1u, bad_proxy, "");
@@ -258,7 +229,7 @@ class HttpNetworkLayerTest : public PlatformTest {
       mock_socket_factory_.AddSocketDataProvider(&data2);
 
     // Expect that we get "Bypass message", and not "content"..
-    ExecuteRequestExpectingContentAndHeader("GET", "Bypass message", "", "");
+    ExecuteRequestExpectingContentAndHeader("Bypass message", "", "");
 
     // We should also observe the bad proxy or proxies in the retry list.
     TestBadProxies(proxy_count, bad_proxy, bad_proxy2);
@@ -367,67 +338,6 @@ TEST_F(HttpNetworkLayerTest, ServerTwoProxyBypassFixed) {
   TestProxyFallback(bad_proxy);
 }
 
-TEST_F(HttpNetworkLayerTest, BypassAndRetryIdempotentMethods) {
-  std::string bad_proxy = GetChromeProxy();
-  const struct {
-    std::string method;
-    std::string content;
-    bool expected_to_retry;
-  } tests[] = {
-    {
-      "GET",
-      "content",
-      true,
-    },
-    {
-      "OPTIONS",
-      "content",
-      true,
-    },
-    {
-      "HEAD",
-      "",
-      true,
-    },
-    {
-      "PUT",
-      "",
-      true,
-    },
-    {
-      "DELETE",
-      "content",
-      true,
-    },
-    {
-      "TRACE",
-      "content",
-      true,
-    },
-    {
-      "POST",
-      "Bypass message",
-      false,
-    },
-  };
-
-  for (size_t i = 0; i < arraysize(tests); ++i) {
-    ConfigureTestDependencies(
-        ProxyService::CreateFixed(bad_proxy +", good:8080"));
-    MockRead data_reads[] = {
-      MockRead("HTTP/1.1 200 OK\r\n"
-               "Chrome-Proxy: bypass=0\r\n\r\n"),
-      MockRead("Bypass message"),
-      MockRead(SYNCHRONOUS, OK),
-    };
-    TestProxyFallbackByMethodWithMockReads(bad_proxy, "", data_reads,
-                                           arraysize(data_reads),
-                                           tests[i].method,
-                                           tests[i].content,
-                                           tests[i].expected_to_retry, 1u);
-  }
-}
-
 TEST_F(HttpNetworkLayerTest, ServerOneProxyWithDirectBypassPac) {
   std::string bad_proxy = GetChromeProxy();
   ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult(
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index d956fb3..a6d5069 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -975,9 +975,9 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
         else
           proxy_bypass_event = ProxyService::LONG_BYPASS;
       } else {
-        // Additionally, fallback if a 500, 502 or 503 is returned via the data
-        // reduction proxy. This is conservative, as the 500, 501 or 502 might
-        // have been generated by the origin, and not the proxy.
+        // Additionally, fallback if a 500 or 502 is returned via the data
+        // reduction proxy. This is conservative, as the 500 or 502 might have
+        // been generated by the origin, and not the proxy.
         if (response_.headers->response_code() == HTTP_INTERNAL_SERVER_ERROR ||
             response_.headers->response_code() == HTTP_BAD_GATEWAY ||
             response_.headers->response_code() == HTTP_SERVICE_UNAVAILABLE) {
@@ -1008,14 +1008,9 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
                                             chrome_proxy_info.bypass_duration,
                                             proxy_server,
                                             net_log_)) {
-          // Only retry idempotent methods. We don't want to resubmit a POST
+          // Only retry in the case of GETs. We don't want to resubmit a POST
           // if the proxy took some action.
-          if (request_->method == "GET" ||
-              request_->method == "OPTIONS" ||
-              request_->method == "HEAD" ||
-              request_->method == "PUT" ||
-              request_->method == "DELETE" ||
-              request_->method == "TRACE") {
+          if (request_->method == "GET") {
             ResetConnectionAndRequestForResend();
             return OK;
           }