OpenURL post data handling.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 612fc59ff8b283cbb988e42110e7476f96f8107f..901e9b73b4737adc5d44ff6821ed4ab277bf4ef9 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -800,6 +800,10 @@ bool IsContentWithCertificateErrorsRelevantToUI(
               ssl_status.connection_status);
 }
 
+bool IsHttpPost(const blink::WebURLRequest& request) {
+  return request.httpMethod().utf8() == "POST";
+}
+
 #if defined(OS_ANDROID)
 // Returns true if WMPI should be used for playback, false otherwise.
 //
@@ -2916,8 +2920,9 @@ void RenderFrameImpl::loadURLExternally(const blink::WebURLRequest& request,
                                                    request.url(), referrer,
                                                    suggested_name));
   } else {
-    OpenURL(request.url(), referrer, policy, should_replace_current_entry,
-            false);
+    OpenURL(request.url(), IsHttpPost(request),
+            GetRequestBodyForWebURLRequest(request), referrer, policy,

[dcheng, 2016/06/12 05:29:00]

Two questions:
1) Is there another CL that changes GetRequestBodyForWebURLRequest()?
GetRequestBodyForWebURLRequest() will currently DCHECK() if the request is a GET
or HEAD request.
2) DO we need to pass both IsHttpPost(Request) and the request body? Can we
infer POST based on request body being non-null?

[Łukasz Anforowicz, 2016/06/13 18:26:23]

There is no pending CL related to the current one + modifying
GetRequestBodyForWebURLRequest.

I think this is fine, because the DCHECK will fire only if *both* conditions are
bet:

A) The request is a GET or HEAD request

B) The request has a body.

That is - we return before the DCHECK if |request.httpBody().isNull()|.
Yeah, I wondered about this.  I think having 2 separate values is better
because:

1. Separation is consistent with 

1.1. chrome::NavigateParams (which has a |bool uses_post| field).

1.2. blink::ResourceRequest and net::URLRequest (which both have a separate
method and body fields).  Granted - these deal not only with page/frame
navigations (e.g. they also deal with other kinds network requests - for example
XHRs can use PUT or other more esoteric HTTP methods), but I think the
consistency is still desirable.

2. IMO separation is desirable for maintainability, because these truly are
separate things - i.e. presence of request body implies HTTP POST method in page
navigations *today*, but this implication might not hold in the future (i.e. if
body will start making sense for non-HTTP navigations, of if we decide to start
supporting PUT for page navigations;  I understand that these examples are
hypothetical, but I hopefully they illustrate why mxing of method+body into one
field might be undesirable / might introduce unnecessary assumptions into code).

+            should_replace_current_entry, false);
   }
 }
 
@@ -4783,8 +4788,9 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
   if (is_content_initiated && IsTopLevelNavigation(frame_) &&
       render_view_->renderer_preferences_
           .browser_handles_all_top_level_requests) {
-    OpenURL(url, referrer, info.defaultPolicy, info.replacesCurrentHistoryItem,
-            false);
+    OpenURL(url, IsHttpPost(info.urlRequest),
+            GetRequestBodyForWebURLRequest(info.urlRequest), referrer,
+            info.defaultPolicy, info.replacesCurrentHistoryItem, false);
     return blink::WebNavigationPolicyIgnore;  // Suppress the load here.
   }
 
@@ -4793,8 +4799,9 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
   // FrameNavigationEntry.  If none is found, fall back to the default url.
   if (SiteIsolationPolicy::UseSubframeNavigationEntries() &&
       info.isHistoryNavigationInNewChildFrame && is_content_initiated) {
-    OpenURL(url, referrer, info.defaultPolicy, info.replacesCurrentHistoryItem,
-            true);
+    OpenURL(url, IsHttpPost(info.urlRequest),
+            GetRequestBodyForWebURLRequest(info.urlRequest), referrer,
+            info.defaultPolicy, info.replacesCurrentHistoryItem, true);
     // Suppress the load in Blink but mark the frame as loading.
     return blink::WebNavigationPolicyHandledByClient;
   }
@@ -4809,15 +4816,6 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
   // an extension or app origin, leaving a WebUI page, etc). We only care about
   // top-level navigations (not iframes). But we sometimes navigate to
   // about:blank to clear a tab, and we want to still allow that.
-  //
-  // Note: this is known to break POST submissions when crossing process
-  // boundaries until http://crbug.com/101395 is fixed.  This is better for
-  // security than loading a WebUI, extension or app page in the wrong process.
-  // POST requests don't work because this mechanism does not preserve form
-  // POST data. We will need to send the request's httpBody data up to the
-  // browser process, and issue a special POST navigation in WebKit (via
-  // FrameLoader::loadFrameRequest). See ResourceDispatcher and WebURLLoaderImpl
-  // for examples of how to send the httpBody data.
   if (!frame_->parent() && is_content_initiated &&
       !url.SchemeIs(url::kAboutScheme)) {
     bool send_referrer = false;
@@ -4857,7 +4855,9 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
     }
 
     if (should_fork) {
-      OpenURL(url, send_referrer ? referrer : Referrer(), info.defaultPolicy,
+      OpenURL(url, IsHttpPost(info.urlRequest),
+              GetRequestBodyForWebURLRequest(info.urlRequest),
+              send_referrer ? referrer : Referrer(), info.defaultPolicy,
               info.replacesCurrentHistoryItem, false);
       return blink::WebNavigationPolicyIgnore;  // Suppress the load here.
     }
@@ -4897,8 +4897,9 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
 
   if (is_fork) {
     // Open the URL via the browser, not via WebKit.
-    OpenURL(url, Referrer(), info.defaultPolicy,
-            info.replacesCurrentHistoryItem, false);
+    OpenURL(url, IsHttpPost(info.urlRequest),
+            GetRequestBodyForWebURLRequest(info.urlRequest), Referrer(),
+            info.defaultPolicy, info.replacesCurrentHistoryItem, false);
     return blink::WebNavigationPolicyIgnore;
   }
 
@@ -5264,13 +5265,18 @@ void RenderFrameImpl::OnSelectPopupMenuItems(
 #endif
 #endif
 
-void RenderFrameImpl::OpenURL(const GURL& url,
-                              const Referrer& referrer,
-                              WebNavigationPolicy policy,
-                              bool should_replace_current_entry,
-                              bool is_history_navigation_in_new_child) {
+void RenderFrameImpl::OpenURL(
+    const GURL& url,
+    bool uses_post,
+    const scoped_refptr<ResourceRequestBodyImpl>& resource_request_body,
+    const Referrer& referrer,
+    WebNavigationPolicy policy,
+    bool should_replace_current_entry,
+    bool is_history_navigation_in_new_child) {
   FrameHostMsg_OpenURL_Params params;
   params.url = url;
+  params.uses_post = uses_post;
+  params.resource_request_body = resource_request_body;
   params.referrer = referrer;
   params.disposition = RenderViewImpl::NavigationPolicyToDisposition(policy);