If a HTTP post to a server returns any redirect code other than 307, then browsers don't preserve the

request method, i.e. they convert the POST request to GET. For 307 redirects browsers preserve redirects.

This CL fixes the following issues :-
1. As per the above description, we reset the method which 
   ensures that we don't generate the post related headers.
   The Post302RedirectGet net test does test this very case.
   However it works correctly as Chrome follows the redirect  
   and reissues the GET request. In this case this does not 
   occur as the only calls which are invoked on the bind 
   status callback after the redirect are GetBindInfo and
   BeginningTransaction where we incorrectly return the post
   related information. Ideally we would want to turn off
   follow redirects in Urlmon or Chrome. I tried the latter
   which has a number of issues. 

2. In debug mode the chrome_frame_net_tests cause a DCHECK 
   to be fired which indicates that the test is not being     
   run on the UI thread.

3. As the Urlmon requests are now destroyed asynchronously 
   having a DCHECK after the Stop call on the Urlmon
   request object in the CleanupAsyncRequests function is 
   incorrect. Removed this DCHECK

Fixes bug http://code.google.com/p/chromium/issues/detail?id=25643

Bug=25643

Review URL: http://codereview.chromium.org/333043

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

5 files changed, 22 insertions, 9 deletions

diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 4529ca3..e02173d 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -1060,7 +1060,6 @@ void ChromeFrameAutomationClient::CleanupAsyncRequests() {
     index++;
   }
 
-  DCHECK(request_map_.empty());
   request_map_.clear();
 }
 
diff --git a/chrome_frame/plugin_url_request.h b/chrome_frame/plugin_url_request.h
index 8c38b92..5e52103 100644
--- a/chrome_frame/plugin_url_request.h
+++ b/chrome_frame/plugin_url_request.h
@@ -76,6 +76,11 @@ class PluginUrlRequest : public UrlRequestReference {
   const std::string& method() const {
     return method_;
   }
+
+  void set_method(const std::string& new_method) {
+    method_ = new_method;
+  }
+
   const std::string& referrer() const {
     return referrer_;
   }
diff --git a/chrome_frame/test/net/fake_external_tab.cc b/chrome_frame/test/net/fake_external_tab.cc
index cc51b3d..760c5b8 100644
--- a/chrome_frame/test/net/fake_external_tab.cc
+++ b/chrome_frame/test/net/fake_external_tab.cc
@@ -19,7 +19,6 @@
 #include "base/scoped_variant_win.h"
 
 #include "chrome/browser/browser_prefs.h"
-#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/process_singleton.h"
 #include "chrome/browser/profile_manager.h"
 #include "chrome/common/chrome_constants.h"
@@ -145,6 +144,9 @@ CFUrlRequestUnittestRunner::CFUrlRequestUnittestRunner(int argc, char** argv)
   pss_subclass_.reset(new ProcessSingletonSubclass(this));
   EXPECT_TRUE(pss_subclass_->Subclass(fake_chrome_.user_data()));
   StartChromeFrameInHostBrowser();
+  // Register the main thread by instantiating it, but don't call any methods.
+  main_thread_.reset(new ChromeThread());
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 }
 
 CFUrlRequestUnittestRunner::~CFUrlRequestUnittestRunner() {
@@ -277,11 +279,6 @@ void CFUrlRequestUnittestRunner::OnInitialTabLoaded() {
 void CFUrlRequestUnittestRunner::RunMainUIThread() {
   DCHECK(MessageLoop::current());
   DCHECK(MessageLoop::current()->type() == MessageLoop::TYPE_UI);
-
-  // Register the main thread by instantiating it, but don't call any methods.
-  ChromeThread main_thread(ChromeThread::UI, MessageLoop::current());
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-
   MessageLoop::current()->Run();
 }
 
diff --git a/chrome_frame/test/net/fake_external_tab.h b/chrome_frame/test/net/fake_external_tab.h
index 6ce4f93..79e45cf 100644
--- a/chrome_frame/test/net/fake_external_tab.h
+++ b/chrome_frame/test/net/fake_external_tab.h
@@ -12,7 +12,7 @@
 
 #include "chrome/app/scoped_ole_initializer.h"
 #include "chrome/browser/browser_process_impl.h"
-
+#include "chrome/browser/chrome_thread.h"
 #include "chrome_frame/test/test_server.h"
 #include "chrome_frame/test/net/test_automation_provider.h"
 #include "chrome_frame/test/net/process_singleton_subclass.h"
@@ -21,7 +21,7 @@
 
 class ProcessSingleton;
 
-class FakeExternalTab { 
+class FakeExternalTab {
  public:
   FakeExternalTab();
   ~FakeExternalTab();
@@ -101,6 +101,7 @@ class CFUrlRequestUnittestRunner
   // on the main thread.
   FakeExternalTab fake_chrome_;
   scoped_ptr<ProcessSingletonSubclass> pss_subclass_;
+  scoped_ptr<ChromeThread> main_thread_;
 };
 
 #endif  // CHROME_FRAME_TEST_NET_FAKE_EXTERNAL_TAB_H_
diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc
index d64cc62..45634b7 100644
--- a/chrome_frame/urlmon_url_request.cc
+++ b/chrome_frame/urlmon_url_request.cc
@@ -168,6 +168,17 @@ STDMETHODIMP UrlmonUrlRequest::OnProgress(ULONG progress, ULONG max_progress,
       // Fetch the redirect status as they aren't all equal (307 in particular
       // retains the HTTP request verb).
       redirect_status_ = GetHttpResponseStatus();
+      // NOTE: Even though RFC 2616 says to preserve the request method when
+      // following a 302 redirect, normal browsers don't do that. Instead they
+      // all convert a POST into a GET in response to a 302 and so shall we.
+      // For 307 redirects, browsers preserve the method.  The RFC says to
+      // prompt the user to confirm the generation of a new POST request, but
+      // IE omits this prompt and so shall we.
+      if (redirect_status_ != 307 &&
+          LowerCaseEqualsASCII(method(), "post")) {
+        set_method("get");
+        post_data_len_ = 0;
+      }
       break;
 
     default: