When cross-site navigations are cancelled, delete the request being transferred

content/browser/frame_host/render_frame_host_manager.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/render_frame_host_manager.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_view_devtools_agent_host.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/debug_urls.h"
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/renderer_host/cross_site_request_transfer.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/browser/webui/web_ui_impl.h"
 #include "content/common/view_messages.h"
 #include "content/port/browser/render_widget_host_view_port.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 
 namespace content {
 
-RenderFrameHostManager::PendingNavigationParams::PendingNavigationParams()
-    : is_transfer(false), frame_id(-1), should_replace_current_entry(false) {
-}
-
 RenderFrameHostManager::PendingNavigationParams::PendingNavigationParams(
     const GlobalRequestID& global_request_id,
-    bool is_transfer,
+    scoped_ptr<CrossSiteRequestTransfer> cross_site_request_transfer,
     const std::vector<GURL>& transfer_url_chain,
     Referrer referrer,
     PageTransition page_transition,
     int64 frame_id,
     bool should_replace_current_entry)
     : global_request_id(global_request_id),
-      is_transfer(is_transfer),
+      cross_site_request_transfer(cross_site_request_transfer.Pass()),
       transfer_url_chain(transfer_url_chain),
       referrer(referrer),
       page_transition(page_transition),
       frame_id(frame_id),
       should_replace_current_entry(should_replace_current_entry) {
 }
 
 RenderFrameHostManager::PendingNavigationParams::~PendingNavigationParams() {}
 
 RenderFrameHostManager::RenderFrameHostManager(
@@ skipping 208 matching lines @@
       !pending_nav_params_.get()) {
     pending_nav_params_.reset();
     return;
   }
 
   // Now that the unload handler has run, we need to either initiate the
   // pending transfer (if there is one) or resume the paused response (if not).
   // TODO(creis): The blank swapped out page is visible during this time, but
   // we can shorten this by delivering the response directly, rather than
   // forcing an identical request to be made.
-  if (pending_nav_params_->is_transfer) {
+  if (pending_nav_params_->cross_site_request_transfer) {
     // Treat the last URL in the chain as the destination and the remainder as
     // the redirect chain.
     CHECK(pending_nav_params_->transfer_url_chain.size());
     GURL transfer_url = pending_nav_params_->transfer_url_chain.back();
     pending_nav_params_->transfer_url_chain.pop_back();
 
     // We don't know whether the original request had |user_action| set to true.
     // However, since we force the navigation to be in the current tab, it
     // doesn't matter.
     render_view_host->GetDelegate()->RequestTransferURL(
@@ skipping 28 matching lines @@
   }
 
   // Sanity check that this is for the correct frame.
   DCHECK_EQ(frame_tree_node_->frame_id(), pending_nav_params_->frame_id);
 
   // Now that the unload handler has run, we need to either initiate the
   // pending transfer (if there is one) or resume the paused response (if not).
   // TODO(creis): The blank swapped out page is visible during this time, but
   // we can shorten this by delivering the response directly, rather than
   // forcing an identical request to be made.
-  if (pending_nav_params_->is_transfer) {
+  if (pending_nav_params_->cross_site_request_transfer) {
     // Treat the last URL in the chain as the destination and the remainder as
     // the redirect chain.
     CHECK(pending_nav_params_->transfer_url_chain.size());
     GURL transfer_url = pending_nav_params_->transfer_url_chain.back();
     pending_nav_params_->transfer_url_chain.pop_back();
 
     // We don't know whether the original request had |user_action| set to true.
     // However, since we force the navigation to be in the current tab, it
     // doesn't matter.
     // TODO(creis): Move RequestTransferURL to RenderFrameHost's navigator.
@@ skipping 131 matching lines @@
       // This is not a cross-site navigation, the tab is being closed.
       render_frame_host_->render_view_host()->ClosePage();
     }
   }
 }
 
 // TODO(creis): Take in a RenderFrameHost from CSRH.
 void RenderFrameHostManager::OnCrossSiteResponse(
     RenderViewHost* pending_render_view_host,
     const GlobalRequestID& global_request_id,
-    bool is_transfer,
+    scoped_ptr<CrossSiteRequestTransfer> cross_site_request_transfer,
     const std::vector<GURL>& transfer_url_chain,
     const Referrer& referrer,
     PageTransition page_transition,
     int64 frame_id,
     bool should_replace_current_entry) {
   // This should be called either when the pending RVH is ready to commit or
   // when we realize that the current RVH's request requires a transfer.
   DCHECK(pending_render_view_host == render_frame_host_->render_view_host() ||
          pending_render_view_host ==
              pending_render_frame_host_->render_view_host());
 
   // TODO(creis): Eventually we will want to check all navigation responses
   // here, but currently we pass information for a transfer if
   // ShouldSwapProcessesForRedirect returned true in the network stack.
   // In that case, we should set up a transfer after the unload handler runs.
-  // If is_transfer is false, we will just run the unload handler and resume.
+  // If |cross_site_request_transfer| is NULL, we will just run the unload
+  // handler and resume.
   pending_nav_params_.reset(new PendingNavigationParams(
-      global_request_id, is_transfer, transfer_url_chain, referrer,
-      page_transition, frame_id, should_replace_current_entry));
+      global_request_id, cross_site_request_transfer.Pass(), transfer_url_chain,
+      referrer, page_transition, frame_id, should_replace_current_entry));
 
   // Run the unload handler of the current page.
   SwapOutOldPage();
 }
 
 void RenderFrameHostManager::SwapOutOldPage() {
   // Should only see this while we have a pending renderer or transfer.
   CHECK(cross_navigation_pending_ || pending_nav_params_.get());
 
   // Tell the renderer to suppress any further modal dialogs so that we can swap
@@ skipping 702 matching lines @@
       ShouldSwapBrowsingInstancesForNavigation(current_entry, &entry);
   if (!is_guest_scheme && (ShouldTransitionCrossSite() || force_swap))
     new_instance = GetSiteInstanceForEntry(entry, current_instance, force_swap);
 
   // If force_swap is true, we must use a different SiteInstance.  If we didn't,
   // we would have two RenderFrameHosts in the same SiteInstance and the same
   // frame, resulting in page_id conflicts for their NavigationEntries.
   if (force_swap)
     CHECK_NE(new_instance, current_instance);
 
+  // If entry includes the request ID of a request that is being transferred,
+  // either the old renderer is about to to reclaim it, or a new one is going

[Charlie Reis, 2014/02/13 22:12:19]

nit: to to

I'm not sure I follow this.  Do you mean cancel or abort by "reclaim"?  Can you
specify where the new renderer claims ownership?  (I think you mean
RDH::UpdateRequestForTransfer?)

Also, is there a reason this code is here?  It's currently in the middle of the
decision of whether to swap processes, but that doesn't seem to matter for it. 
If you want it to happen on any navigation that passes through RFHM, maybe
RFHM::Navigate is a better spot?

[mmenke, 2014/02/14 16:30:02]

I mean the current_instance can take ownership - I assume this happens in the
case where we think we have a cross-process navigation, but it turns out we
don't, so the first renderer takes back ownership of the request.  I've
simplified the comment.
You're right...  I had thought this should only run on cross-site navigations,
and since we had special logic for that case here, I put it here.  But then I
found out we sometimes transfer the request to current_instance.  By moving it,
we also now handle the case where CreateRenderFrame fails.

+  // claim ownership, so release ownership of the request.
+  if (pending_nav_params_ &&
+      pending_nav_params_->global_request_id ==
+          entry.transferred_global_request_id()) {
+    pending_nav_params_->cross_site_request_transfer->ReleaseRequest();
+  }
+
   if (new_instance != current_instance) {
     // New SiteInstance: create a pending RFH to navigate.
     DCHECK(!cross_navigation_pending_);
 
     // This will possibly create (set to NULL) a Web UI object for the pending
     // page. We'll use this later to give the page special access. This must
     // happen before the new renderer is created below so it will get bindings.
     // It must also happen after the above conditional call to CancelPending(),
     // otherwise CancelPending may clear the pending_web_ui_ and the page will
     // not have its bindings set appropriately.
@@ skipping 206 matching lines @@
     SiteInstance* instance) const {
   RenderFrameHostMap::const_iterator iter =
       swapped_out_hosts_.find(instance->GetId());
   if (iter != swapped_out_hosts_.end())
     return iter->second;
 
   return NULL;
 }
 
 }  // namespace content