To fix the cross-site post submission bug.

content/renderer/render_view_impl.cc

Index: content/renderer/render_view_impl.cc
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index c834903a34aa5cfc2684a7ff3ede3c831418da40..d3d79b35f0b0f3d45c6892c15bbe815ed0e2f616 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -4,6 +4,8 @@
 
 #include "content/renderer/render_view_impl.h"
 
+#include "webkit/glue/weburlloader_impl.h"
+
 #include <algorithm>
 #include <cmath>
 
@@ -185,6 +187,7 @@
 #include "webkit/glue/alt_error_page_resource_fetcher.h"
 #include "webkit/glue/dom_operations.h"
 #include "webkit/glue/glue_serialize.h"
+#include "webkit/glue/resource_request_body.h"
 #include "webkit/glue/web_intent_service_data.h"
 #include "webkit/glue/webdropdata.h"
 #include "webkit/glue/webkit_constants.h"
@@ -320,6 +323,7 @@ using base::TimeDelta;
 
 using webkit_glue::AltErrorPageResourceFetcher;
 using webkit_glue::ResourceFetcher;
+using webkit_glue::ResourceRequestBody;
 using webkit_glue::WebPreferences;
 using webkit_glue::WebURLResponseExtraDataImpl;
 
@@ -329,9 +333,17 @@ using WebKit::WebFloatPoint;
 using WebKit::WebFloatRect;
 using WebKit::WebHitTestResult;
 #endif
+using content::ViewMsg_Request;
 
 namespace content {
 
+  // Will find a proper place to move to.
+  ViewMsg_Request::ViewMsg_Request() {
+  }
+
+  ViewMsg_Request::~ViewMsg_Request() {
+  }
+
 //-----------------------------------------------------------------------------
 
 typedef std::map<WebKit::WebView*, RenderViewImpl*> ViewMap;
@@ -1132,22 +1144,60 @@ void RenderViewImpl::OnNavigate(const ViewMsg_Navigate_Params& params) {
       }
     }
 
-    if (params.is_post) {
-      request.setHTTPMethod(WebString::fromUTF8("POST"));
-
-      // Set post data.
+    // Deal With Cross-Process Post Submission
+    if(params.is_post) {
       WebHTTPBody http_body;
       http_body.initialize();
-      http_body.appendData(WebData(
-          reinterpret_cast<const char*>(
-              &params.browser_initiated_post_data.front()),
-          params.browser_initiated_post_data.size()));
+      const std::vector<ResourceRequestBody::Element>* uploads =
+          params.request.request_body->elements();
+      std::vector<ResourceRequestBody::Element>::const_iterator iter;
+      for (iter = uploads->begin(); iter != uploads->end(); ++iter) {
+        switch (iter->type()) {
+          case ResourceRequestBody::Element::TYPE_BYTES: {
+            http_body.appendData(WebData(iter->bytes(),
+                                         static_cast<int>(iter->length())));
+            break;
+          }
+          case ResourceRequestBody::Element::TYPE_FILE: {
+            http_body.appendFileRange(
+                WebString::fromUTF8(iter->path().value()),
+                static_cast<long long>(iter->offset()),
+                static_cast<long long>(iter->length()),
+                iter->expected_modification_time().ToDoubleT());
+            break;
+          }
+          case ResourceRequestBody::Element::TYPE_FILE_FILESYSTEM: {
+            // Since I cannot found a cross-site post submission
+            // in this type which does not use XHR,
+            // this part have't been testes.

[michaeln, 2012/11/02 00:08:23]

Q: do cross-site posts ever cross storage partition boundaries?

[irobert, 2012/11/02 17:22:55]

I need to chat with Albert about it.

On 2012/11/02 00:08:23, michaeln wrote:

+            http_body.appendURLRange(
+                iter->url(),
+                static_cast<long long>(iter->offset()),
+                static_cast<long long>(iter->length()),
+                iter->expected_modification_time().ToDoubleT());
+            break;
+          }
+          case ResourceRequestBody::Element:: TYPE_BLOB: {
+            // Since I cannot found a cross-site post submission
+            // in this type which does not use XHR,
+            // this part have't been testes.
+            http_body.appendBlob(iter->url());
+            break;
+          }
+          default:
+            NOTREACHED();
+        }
+      }
       request.setHTTPBody(http_body);
+      request.setHTTPMethod(WebString::fromUTF8(params.request.method));
+      std::string type = params.request.headers.find("Content-Type")->second;
+      request.setHTTPHeaderField(
+          WebString::fromUTF8("Content-Type"),
+          WebString::fromUTF8(type));
     }
 
     main_frame->loadRequest(request);
   }
-
   // In case LoadRequest failed before DidCreateDataSource was called.
   pending_navigation_params_.reset();
 }
@@ -1664,6 +1714,23 @@ void RenderViewImpl::OpenURL(WebFrame* frame,
       frame->identifier()));
 }
 
+// Not sure whether to change the original OpenURL API by adding a
+// new argument. To avoid too much change, use OpenPostURL API do
+// send ViewHostMsg_OpenPostURL message to browser process.
+void RenderViewImpl::OpenPostURL(WebFrame* frame,
+                                 const GURL& url,
+                                 const Referrer& referrer,
+                                 WebNavigationPolicy policy,
+                                 const ViewMsg_Request& request) {
+  Send(new ViewHostMsg_OpenPostURL(
+      routing_id_,
+      url,
+      referrer,
+      NavigationPolicyToDisposition(policy),
+      frame->identifier(),
+      request));
+}
+
 // WebViewDelegate ------------------------------------------------------------
 
 void RenderViewImpl::LoadNavigationErrorPage(
@@ -2724,8 +2791,78 @@ WebNavigationPolicy RenderViewImpl::decidePolicyForNavigation(
     if (!net::RegistryControlledDomainService::SameDomainOrHost(frame_url,
                                                                 url) ||
         frame_url.scheme() != url.scheme()) {
-      OpenURL(frame, url, referrer, default_policy);
-      return WebKit::WebNavigationPolicyIgnore;
+      WebString method = request.httpMethod();
+      if(method != WebString("POST")) {
+        OpenURL(frame, url, referrer, default_policy);
+        return WebKit::WebNavigationPolicyIgnore;
+      }
+      else {

[michaeln, 2012/11/02 00:08:23]

style nit : } else {

[irobert, 2012/11/02 17:22:55]

Done.

+        ViewMsg_Request params;
+
+        // Convert WebHTTPBody to ResourceRequestBody
+        scoped_refptr<ResourceRequestBody> request_body =
+            new ResourceRequestBody();
+        WebHTTPBody body = request.httpBody();
+        WebKit::WebHTTPBody::Element element;
+        for (int i=0; body.elementAt(i, element); i++) {
+          switch (element.type) {
+            case WebHTTPBody::Element::TypeData:
+              if (!element.data.isEmpty())
+                request_body->AppendBytes(
+                    element.data.data(), static_cast<int>(element.data.size()));
+              break;
+            case WebHTTPBody::Element::TypeFile: {
+              #if defined(OS_POSIX)
+                const FilePath::StringType kFilePath = FILE_PATH_LITERAL(

[michaeln, 2012/11/02 00:08:23]

use of FILE_PATH_LITERAL seems odd since the value here is not a literal?

[irobert, 2012/11/02 17:22:55]

Also looks odd to me. But I just followed the usage in the file
resource_request_body_unittest.cc:24.

On 2012/11/02 00:08:23, michaeln wrote:

+                    base::SysWideToNativeMB(UTF16ToWideHack(element.filePath)));
+              #elif defined(OS_WIN)
+                const FilePath::StringType kFilePath = FILE_PATH_LITERAL(
+                    UTF16ToWideHack(element.filePath));
+              #endif
+              if (element.fileLength == -1) {
+                request_body->AppendFileRange(
+                    FilePath(kFilePath), 0, kuint64max, base::Time());
+              } else {
+                request_body->AppendFileRange(
+                    FilePath(kFilePath),
+                    static_cast<uint64>(element.fileStart),
+                    static_cast<uint64>(element.fileLength),
+                    base::Time::FromDoubleT(element.modificationTime));
+              }
+              break;
+            }
+            case WebHTTPBody::Element::TypeURL: {
+              GURL gurl = GURL(element.url);
+              DCHECK(gurl.SchemeIsFileSystem());
+              request_body->AppendFileSystemFileRange(
+                  gurl,
+                  static_cast<uint64>(element.fileStart),
+                  static_cast<uint64>(element.fileLength),
+                  base::Time::FromDoubleT(element.modificationTime));
+              break;
+            }
+            case WebHTTPBody::Element::TypeBlob:
+              request_body->AppendBlob(GURL(element.blobURL));
+              break;
+            default:
+              NOTREACHED();
+          }
+          request_body->set_identifier(body.identifier());
+        }
+        params.request_body = request_body;
+        params.method = "POST";
+        params.url = url;
+        // Extract Header Info.
+        WebString ContentType =
+            request.httpHeaderField(WebString::fromUTF8("Content-Type"));
+        std::string ContentTypeStr (ContentType.utf8().data(),
+                                    ContentType.utf8().length());
+        params.headers.insert(std::pair<std::string,std::string>(

[michaeln, 2012/11/02 00:08:23]

if there's at most one header value in here, maybe we don't need a whole headers
struct for it and instead could just sent contentType as an explicit msg param

[irobert, 2012/11/02 17:22:55]

For Cross-Process Post Submission, I think Content-Type is the only important
header. The reason why I did not use explicit msg param is that I think it can
be reused for other problems which need to send all the header through IPC.

On 2012/11/02 00:08:23, michaeln wrote:

+                                  "Content-Type", ContentTypeStr));
+
+        OpenPostURL(frame, url, referrer, default_policy, params);
+        return default_policy;

[michaeln, 2012/11/02 00:08:23]

return default_policy? Is that right?

Seems like this s/b WebKit::WebNavigationPolicyIgnor since by calling
OpenPostURL we've just asked the browser process to handle this post in a
different renderer. By returning default, does will this POST happens twice,
once in this renderer and again in the other renderer per OpenPostURL
processing?

[irobert, 2012/11/02 17:22:55]

Yes. return WebKit::WebNavigationPolicyIgnor will break the cross-site Post
submission. I did not notice the POST happens twice. But I am still
investigating what is the right value to return.

On 2012/11/02 00:08:23, michaeln wrote:

[Charlie Reis, 2012/11/05 16:21:40]

Ignore is the correct thing to return.  There must be some other issue if that
breaks the submission, because returning the default policy means the old
renderer will continue with its POST submission as well.

+      }
     }
   }