Forwarding POST body into renderer after a cross-site transfer.

content/browser/frame_host/navigator_impl.cc

Index: content/browser/frame_host/navigator_impl.cc
diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
index 6fcdb074b4a0f6badd1ea04256214a7e2223480d..7d9be13462808959491737ed2611104061a4fece 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -378,7 +378,8 @@ bool NavigatorImpl::NavigateToEntry(
           entry.ConstructCommonNavigationParams(dest_url, dest_referrer,
                                                 navigation_type, lofi_state,
                                                 navigation_start),
-          entry.ConstructStartNavigationParams(),
+          entry.ConstructStartNavigationParams(
+              *dest_render_frame_host->navigation_handle()),

[clamy, 2016/05/12 05:32:48]

This is unsafe. In non transfer cases the dest RFH does not have a
NavigationHandle at that point.

[Łukasz Anforowicz, 2016/05/17 23:52:56]

Done.  In subsequent patchsets I've added |const NavigationHandleImpl*
transfer_navigation_handle| parameter to RequestTransferURL method.

           entry.ConstructRequestNavigationParams(
               frame_entry, is_same_document_history_load,
               frame_tree_node->has_committed_real_load(),
@@ -771,12 +772,22 @@ void NavigatorImpl::RequestTransferURL(
               is_renderer_initiated, std::string(),
               controller_->GetBrowserContext()));
     }
+    // TODO(lukasza): DO NOT SUBMIT: This seems very wrong:
+    // 1. This ignores methods other than GET and POST
+    // 2. This feels like a wrong place to do stuff.
+    // 3. This doesn't touch PageState at all...
+    bool is_post = static_cast<bool>(

[clamy, 2016/05/12 05:32:48]

Suggestion: add the method as a parameter of RequestTransferURL. The caller,
RFHM::OnCrossSiteResponse has access to the NavigationHandle which stores it.

[Łukasz Anforowicz, 2016/05/17 23:52:56]

I've ended up passing |const NavigationHandleImpl* transfer_navigation_handle|
as a parameter of RequestTransferURL method.  NavigationHandle lets me access
both 1) HTTP method and 2) POST body.

+        render_frame_host->navigation_handle()->resource_request_body());
+
     // TODO(creis): Handle POST submissions.  See https://crbug.com/582211 and
     // https://crbug.com/101395.
     entry->AddOrUpdateFrameEntry(
         node, -1, -1, nullptr,
         static_cast<SiteInstanceImpl*>(source_site_instance), dest_url,
-        referrer_to_use, PageState(), "GET", -1);
+        referrer_to_use,
+        // TODO(lukasza): DO NOT SUBMIT: Should I populate a real PageState here
+        // (using NavigationHandleImpl::resource_request_body() I've introduced)

[clamy, 2016/05/12 05:32:48]

If you populate the PageState the RenderFrame will interpret it as a history
navigation. We check whether the PageState is valid at the beginning of
RenderFrameImpl::NavigateInternal to determine if this is a history navigation
or not. This is not the behavior we want.

[Łukasz Anforowicz, 2016/05/17 23:52:56]

Acknowledged.

+        PageState(), is_post ? "POST" : "GET", -1);
   } else {
     // Main frame case.
     entry = NavigationEntryImpl::FromNavigationEntry(
@@ -807,6 +818,7 @@ void NavigatorImpl::RequestTransferURL(
   scoped_refptr<FrameNavigationEntry> frame_entry(entry->GetFrameEntry(node));
   if (!frame_entry) {
     // TODO(creis): Handle POST submissions here, as above.
+    // TODO(lukasza): DO NOT SUBMIT: Need to do something here as well.
     frame_entry = new FrameNavigationEntry(
         node->unique_name(), -1, -1, nullptr,
         static_cast<SiteInstanceImpl*>(source_site_instance), dest_url,