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

content/browser/frame_host/navigator_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/navigator_impl.h"
 
 #include <utility>
 
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
@@ skipping 360 matching lines @@
     if (!is_transfer_to_same) {
       navigation_data_.reset(new NavigationMetricsData(
           navigation_start, dest_url, entry.restore_type()));
       // Create the navigation parameters.
       FrameMsg_Navigate_Type::Value navigation_type = GetNavigationType(
           controller_->GetBrowserContext(), entry, reload_type);
       dest_render_frame_host->Navigate(
           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(),
               controller_->GetPendingEntryIndex() == -1,
               controller_->GetIndexOfEntry(&entry),
               controller_->GetLastCommittedEntryIndex(),
               controller_->GetEntryCount()));
     } else {
       // No need to navigate again.  Just resume the deferred request.
       dest_render_frame_host->GetProcess()->ResumeDeferredNavigation(
@@ skipping 372 matching lines @@
     } else {
       // If there's no last committed entry, create an entry for about:blank
       // with a subframe entry for our destination.
       // TODO(creis): Ensure this case can't exist in https://crbug.com/524208.
       entry = NavigationEntryImpl::FromNavigationEntry(
           controller_->CreateNavigationEntry(
               GURL(url::kAboutBlankURL), referrer_to_use, page_transition,
               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(
         controller_->CreateNavigationEntry(
             dest_url, referrer_to_use, page_transition, is_renderer_initiated,
             std::string(), controller_->GetBrowserContext()));
     entry->root_node()->frame_entry->set_source_site_instance(
         static_cast<SiteInstanceImpl*>(source_site_instance));
   }
 
@@ skipping 10 matching lines @@
   // TODO(creis): Set user gesture and intent received timestamp on Android.
 
   // We may not have successfully added the FrameNavigationEntry to |entry|
   // above (per https://crbug.com/608402), in which case we create it from
   // scratch.  This works because we do not depend on |frame_entry| being inside
   // |entry| during NavigateToEntry.  This will go away when we shortcut this
   // further in https://crbug.com/536906.
   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,
         referrer_to_use, "GET", -1);
   }
   NavigateToEntry(node, *frame_entry, *entry.get(),
                   NavigationController::NO_RELOAD, false, false);
 }
 
 // PlzNavigate
@@ skipping 327 matching lines @@
   if (pending_entry != controller_->GetVisibleEntry() ||
       !should_preserve_entry) {
     controller_->DiscardPendingEntry(true);
 
     // Also force the UI to refresh.
     controller_->delegate()->NotifyNavigationStateChanged(INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content