Expose the method used for the next URLRequest redirect.

This makes it possible for URLRequest client code to determine
the method that will be used to resolve the next step in the redirect
chain.
This is to fix android_webview not doing navigation intercepting
if the result of a POST resulted in a 302 redirect.

BUG=b/9773847
TEST=AndroidWebViewTest, net_unittests

Review URL: https://chromiumcodereview.appspot.com/18429010

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

7 files changed, 153 insertions, 26 deletions

diff --git a/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc b/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
index 34b6ada..2e59cd5 100644
--- a/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
+++ b/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
@@ -212,8 +212,6 @@ void AwResourceDispatcherHostDelegate::RequestBeginning(
       child_id, route_id, request));
 
   bool allow_intercepting =
-      // We ignore POST requests because of BUG=155250.
-      request->method() != "POST" &&
       // We allow intercepting navigations within subframes, but only if the
       // scheme other than http or https. This is because the embedder
       // can't distinguish main frame and subframe callbacks (which could lead
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
index 09d9cbe..9f3b475 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
@@ -619,6 +619,43 @@ public class AwContentsClientShouldOverrideUrlLoadingTest extends AwTestBase {
 
     @SmallTest
     @Feature({"AndroidWebView", "Navigation"})
+    public void testCalledFor302AfterPostNavigations() throws Throwable {
+        // The reason POST requests are excluded is BUG 155250.
+        final TestAwContentsClient contentsClient = new TestAwContentsClient();
+        final AwTestContainerView testContainerView =
+            createAwTestContainerViewOnMainSync(contentsClient);
+        final AwContents awContents = testContainerView.getAwContents();
+        final TestAwContentsClient.ShouldOverrideUrlLoadingHelper shouldOverrideUrlLoadingHelper =
+            contentsClient.getShouldOverrideUrlLoadingHelper();
+
+        final String redirectTargetUrl = createRedirectTargetPage(mWebServer);
+        final String postToGetRedirectUrl = mWebServer.setRedirect("/302.html", redirectTargetUrl);
+        final String postLinkUrl = addPageToTestServer(mWebServer, "/page_with_post_link.html",
+                getHtmlForPageWithSimplePostFormTo(postToGetRedirectUrl));
+
+        loadUrlSync(awContents, contentsClient.getOnPageFinishedHelper(), postLinkUrl);
+
+        final int shouldOverrideUrlLoadingCallCount =
+            shouldOverrideUrlLoadingHelper.getCallCount();
+
+        clickOnLinkUsingJs(awContents, contentsClient);
+
+        shouldOverrideUrlLoadingHelper.waitForCallback(shouldOverrideUrlLoadingCallCount);
+
+        // Wait for the target URL to be fetched from the server.
+        assertTrue(CriteriaHelper.pollForCriteria(new Criteria() {
+            @Override
+            public boolean isSatisfied() {
+                return mWebServer.getRequestCount(REDIRECT_TARGET_PATH) == 1;
+            }
+        }, WAIT_TIMEOUT_SECONDS * 1000L, CHECK_INTERVAL));
+
+        assertEquals(redirectTargetUrl,
+                shouldOverrideUrlLoadingHelper.getShouldOverrideUrlLoadingUrl());
+    }
+
+    @SmallTest
+    @Feature({"AndroidWebView", "Navigation"})
     public void testNotCalledForIframeHttpNavigations() throws Throwable {
         final TestAwContentsClient contentsClient = new TestAwContentsClient();
         final AwTestContainerView testContainerView =
diff --git a/components/navigation_interception/intercept_navigation_resource_throttle.cc b/components/navigation_interception/intercept_navigation_resource_throttle.cc
index 390be67..73513ef 100644
--- a/components/navigation_interception/intercept_navigation_resource_throttle.cc
+++ b/components/navigation_interception/intercept_navigation_resource_throttle.cc
@@ -13,6 +13,7 @@
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/common/page_transition_types.h"
 #include "content/public/common/referrer.h"
+#include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
 
 using content::BrowserThread;
@@ -68,17 +69,28 @@ InterceptNavigationResourceThrottle::~InterceptNavigationResourceThrottle() {
 }
 
 void InterceptNavigationResourceThrottle::WillStartRequest(bool* defer) {
-  *defer = CheckIfShouldIgnoreNavigation(request_->url(), false);
+  *defer =
+      CheckIfShouldIgnoreNavigation(request_->url(), request_->method(), false);
 }
 
 void InterceptNavigationResourceThrottle::WillRedirectRequest(
     const GURL& new_url,
     bool* defer) {
-  *defer = CheckIfShouldIgnoreNavigation(new_url, true);
+  *defer =
+      CheckIfShouldIgnoreNavigation(new_url, GetMethodAfterRedirect(), true);
+}
+
+std::string InterceptNavigationResourceThrottle::GetMethodAfterRedirect() {
+  net::HttpResponseHeaders* headers = request_->response_headers();
+  if (!headers)
+    return request_->method();
+  return net::URLRequest::ComputeMethodForRedirect(
+             request_->method(), headers->response_code());
 }
 
 bool InterceptNavigationResourceThrottle::CheckIfShouldIgnoreNavigation(
     const GURL& url,
+    const std::string& method,
     bool is_redirect) {
   const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request_);
   if (!info)
@@ -92,7 +104,7 @@ bool InterceptNavigationResourceThrottle::CheckIfShouldIgnoreNavigation(
                                      Referrer(GURL(request_->referrer()),
                                               info->GetReferrerPolicy()),
                                      info->HasUserGesture(),
-                                     request_->method() == "POST",
+                                     method == "POST",
                                      info->GetPageTransition(),
                                      is_redirect);
 
diff --git a/components/navigation_interception/intercept_navigation_resource_throttle.h b/components/navigation_interception/intercept_navigation_resource_throttle.h
index 012cd568..79ed57e 100644
--- a/components/navigation_interception/intercept_navigation_resource_throttle.h
+++ b/components/navigation_interception/intercept_navigation_resource_throttle.h
@@ -44,7 +44,10 @@ class InterceptNavigationResourceThrottle : public content::ResourceThrottle {
   virtual void WillRedirectRequest(const GURL& new_url, bool* defer) OVERRIDE;
 
  private:
-  bool CheckIfShouldIgnoreNavigation(const GURL& url, bool is_redirect);
+  std::string GetMethodAfterRedirect();
+  bool CheckIfShouldIgnoreNavigation(const GURL& url,
+                                     const std::string& method,
+                                     bool is_redirect);
   void OnResultObtained(bool should_ignore_navigation);
 
   net::URLRequest* request_;
diff --git a/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc b/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc
index 6082e0f..dfccb30 100644
--- a/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc
+++ b/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc
@@ -23,6 +23,8 @@
 #include "content/public/common/page_transition_types.h"
 #include "content/public/test/mock_resource_context.h"
 #include "content/public/test/test_renderer_host.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_response_info.h"
 #include "net/url_request/url_request.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -102,12 +104,18 @@ class MockResourceController : public content::ResourceController {
 
 // TestIOThreadState ----------------------------------------------------------
 
+enum RedirectMode {
+  REDIRECT_MODE_NO_REDIRECT,
+  REDIRECT_MODE_302,
+};
+
 class TestIOThreadState {
  public:
   TestIOThreadState(const GURL& url,
                     int render_process_id,
                     int render_view_id,
                     const std::string& request_method,
+                    RedirectMode redirect_mode,
                     MockInterceptCallbackReceiver* callback_receiver)
       : request_(url, NULL, resource_context_.GetRequestContext()) {
     DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
@@ -126,6 +134,13 @@ class TestIOThreadState {
                      base::Unretained(callback_receiver))));
       throttle_->set_controller_for_testing(&throttle_controller_);
       request_.set_method(request_method);
+
+      if (redirect_mode == REDIRECT_MODE_302) {
+        net::HttpResponseInfo& response_info =
+            const_cast<net::HttpResponseInfo&>(request_.response_info());
+        response_info.headers = new net::HttpResponseHeaders(
+            "Status: 302 Found\0\0");
+      }
   }
 
   void ThrottleWillStartRequest(bool* defer) {
@@ -133,6 +148,11 @@ class TestIOThreadState {
     throttle_->WillStartRequest(defer);
   }
 
+  void ThrottleWillRedirectRequest(const GURL& new_url, bool* defer) {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+    throttle_->WillRedirectRequest(new_url, defer);
+  }
+
   bool request_resumed() const {
     return throttle_controller_.status() ==
         MockResourceController::RESUMED;
@@ -183,16 +203,22 @@ class InterceptNavigationResourceThrottleTest
   void RunThrottleWillStartRequestOnIOThread(
       const GURL& url,
       const std::string& request_method,
+      RedirectMode redirect_mode,
       int render_process_id,
       int render_view_id,
       bool* defer) {
     DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
     TestIOThreadState* io_thread_state =
         new TestIOThreadState(url, render_process_id, render_view_id,
-                              request_method, mock_callback_receiver_.get());
+                              request_method, redirect_mode,
+                              mock_callback_receiver_.get());
 
     SetIOThreadState(io_thread_state);
-    io_thread_state->ThrottleWillStartRequest(defer);
+
+    if (redirect_mode == REDIRECT_MODE_NO_REDIRECT)
+      io_thread_state->ThrottleWillStartRequest(defer);
+    else
+      io_thread_state->ThrottleWillRedirectRequest(url, defer);
   }
 
  protected:
@@ -220,6 +246,7 @@ class InterceptNavigationResourceThrottleTest
             base::Unretained(this),
             GURL(kTestUrl),
             "GET",
+            REDIRECT_MODE_NO_REDIRECT,
             web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
             web_contents()->GetRenderViewHost()->GetRoutingID(),
             base::Unretained(defer)));
@@ -284,6 +311,7 @@ TEST_F(InterceptNavigationResourceThrottleTest,
           base::Unretained(this),
           GURL(kTestUrl),
           "GET",
+          REDIRECT_MODE_NO_REDIRECT,
           web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
           web_contents()->GetRenderViewHost()->GetRoutingID(),
           base::Unretained(&defer)));
@@ -317,6 +345,7 @@ TEST_F(InterceptNavigationResourceThrottleTest,
           base::Unretained(this),
           GURL(kTestUrl),
           "GET",
+          REDIRECT_MODE_NO_REDIRECT,
           MSG_ROUTING_NONE,
           MSG_ROUTING_NONE,
           base::Unretained(&defer)));
@@ -347,6 +376,7 @@ TEST_F(InterceptNavigationResourceThrottleTest,
           base::Unretained(this),
           GURL(kUnsafeTestUrl),
           "GET",
+          REDIRECT_MODE_NO_REDIRECT,
           web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
           web_contents()->GetRenderViewHost()->GetRoutingID(),
           base::Unretained(&defer)));
@@ -374,6 +404,7 @@ TEST_F(InterceptNavigationResourceThrottleTest,
           base::Unretained(this),
           GURL(kTestUrl),
           "GET",
+          REDIRECT_MODE_NO_REDIRECT,
           web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
           web_contents()->GetRenderViewHost()->GetRoutingID(),
           base::Unretained(&defer)));
@@ -401,6 +432,35 @@ TEST_F(InterceptNavigationResourceThrottleTest,
           base::Unretained(this),
           GURL(kTestUrl),
           "POST",
+          REDIRECT_MODE_NO_REDIRECT,
+          web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
+          web_contents()->GetRenderViewHost()->GetRoutingID(),
+          base::Unretained(&defer)));
+
+  // Wait for the request to finish processing.
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(InterceptNavigationResourceThrottleTest,
+       CallbackIsPostFalseForPostConvertedToGetBy302) {
+  bool defer = false;
+
+  EXPECT_CALL(*mock_callback_receiver_,
+              ShouldIgnoreNavigation(_, AllOf(
+                  NavigationParamsUrlIsSafe(),
+                  Property(&NavigationParams::is_post, Eq(false)))))
+      .WillOnce(Return(false));
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(
+          &InterceptNavigationResourceThrottleTest::
+              RunThrottleWillStartRequestOnIOThread,
+          base::Unretained(this),
+          GURL(kTestUrl),
+          "POST",
+          REDIRECT_MODE_302,
           web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
           web_contents()->GetRenderViewHost()->GetRoutingID(),
           base::Unretained(&defer)));
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 3e3dbd1..2d0ce67 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -489,6 +489,26 @@ void URLRequest::set_method(const std::string& method) {
   method_ = method;
 }
 
+// static
+std::string URLRequest::ComputeMethodForRedirect(
+    const std::string& method,
+    int http_status_code) {
+  // For 303 redirects, all request methods except HEAD are converted to GET,
+  // as per the latest httpbis draft.  The draft also allows POST requests to
+  // be converted to GETs when following 301/302 redirects, for historical
+  // reasons. Most major browsers do this and so shall we.  Both RFC 2616 and
+  // the httpbis draft say to prompt the user to confirm the generation of new
+  // requests, other than GET and HEAD requests, but IE omits these prompts and
+  // so shall we.
+  // See:  https://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-17#section-7.3
+  if ((http_status_code == 303 && method != "HEAD") ||
+      ((http_status_code == 301 || http_status_code == 302) &&
+       method == "POST")) {
+    return "GET";
+  }
+  return method;
+}
+
 void URLRequest::SetReferrer(const std::string& referrer) {
   DCHECK(!is_pending_);
   referrer_ = referrer;
@@ -843,27 +863,18 @@ int URLRequest::Redirect(const GURL& location, int http_status_code) {
     final_upload_progress_ = job_->GetUploadProgress();
   PrepareToRestart();
 
-  // For 303 redirects, all request methods except HEAD are converted to GET,
-  // as per the latest httpbis draft.  The draft also allows POST requests to
-  // be converted to GETs when following 301/302 redirects, for historical
-  // reasons. Most major browsers do this and so shall we.  Both RFC 2616 and
-  // the httpbis draft say to prompt the user to confirm the generation of new
-  // requests, other than GET and HEAD requests, but IE omits these prompts and
-  // so shall we.
-  // See:  https://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-17#section-7.3
-  bool was_post = method_ == "POST";
-  if ((http_status_code == 303 && method_ != "HEAD") ||
-      ((http_status_code == 301 || http_status_code == 302) && was_post)) {
-    method_ = "GET";
-    upload_data_stream_.reset();
-    if (was_post) {
-      // If being switched from POST to GET, must remove headers that were
-      // specific to the POST and don't have meaning in GET. For example
-      // the inclusion of a multipart Content-Type header in GET can cause
-      // problems with some servers:
+  std::string new_method(ComputeMethodForRedirect(method_, http_status_code));
+  if (new_method != method_) {
+    if (method_ == "POST") {
+      // If being switched from POST, must remove headers that were specific to
+      // the POST and don't have meaning in other methods. For example the
+      // inclusion of a multipart Content-Type header in GET can cause problems
+      // with some servers:
       // http://code.google.com/p/chromium/issues/detail?id=843
       StripPostSpecificHeaders(&extra_request_headers_);
     }
+    upload_data_stream_.reset();
+    method_.swap(new_method);
   }
 
   // Suppress the referrer if we're redirecting out of https.
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index 45a3f11..03978d6 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -365,6 +365,12 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   const std::string& method() const { return method_; }
   void set_method(const std::string& method);
 
+  // Determines the new method of the request afer following a redirect.
+  // |method| is the method used to arrive at the redirect,
+  // |http_status_code| is the status code associated with the redirect.
+  static std::string ComputeMethodForRedirect(const std::string& method,
+                                              int http_status_code);
+
   // The referrer URL for the request.  This header may actually be suppressed
   // from the underlying network request for security reasons (e.g., a HTTPS
   // URL will not be sent as the referrer for a HTTP request).  The referrer