Move TransferNavigationResourceThrottle into CrossSiteResourceHandler.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 4d36568e34f5d38e395adc3f3731546306365625..79a4d453627baf46723fdc38b65e60ac9d1c79e5 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -109,34 +109,42 @@
 // a new RenderViewHost dedicated to the new SiteInstance.  This works as
 // follows:
 //
-// - Navigate determines whether the destination is cross-site, and if so,
+// - RVHM::Navigate determines whether the destination is cross-site, and if so,
 //   it creates a pending_render_view_host_.
 // - The pending RVH is "suspended," so that no navigation messages are sent to
-//   its renderer until the onbeforeunload JavaScript handler has a chance to
+//   its renderer until the beforeunload JavaScript handler has a chance to
 //   run in the current RVH.
 // - The pending RVH tells CrossSiteRequestManager (a thread-safe singleton)
-//   that it has a pending cross-site request.  ResourceDispatcherHost will
-//   check for this when the response arrives.
-// - The current RVH runs its onbeforeunload handler.  If it returns false, we
+//   that it has a pending cross-site request.  We will check this on the IO
+//   thread when deciding how to handle the response.
+// - The current RVH runs its beforeunload handler.  If it returns false, we
 //   cancel all the pending logic.  Otherwise we allow the pending RVH to send
 //   the navigation request to its renderer.
 // - ResourceDispatcherHost receives a ResourceRequest on the IO thread for the
-//   main resource load on the pending RVH. It checks CrossSiteRequestManager
-//   to see that it is a cross-site request, and installs a
-//   CrossSiteResourceHandler.
+//   main resource load on the pending RVH.  It creates a
+//   CrossSiteResourceHandler to check whether a process swap is needed when
+//   the request is ready to commit.
 // - When RDH receives a response, the BufferedResourceHandler determines
 //   whether it is a download.  If so, it sends a message to the new renderer
 //   causing it to cancel the request, and the download proceeds. For now, the
 //   pending RVH remains until the next DidNavigate event for this
 //   WebContentsImpl. This isn't ideal, but it doesn't affect any functionality.
 // - After RDH receives a response and determines that it is safe and not a
-//   download, it pauses the response to first run the old page's onunload
-//   handler.  It does this by asynchronously calling the OnCrossSiteResponse
-//   method of WebContentsImpl on the UI thread, which sends a SwapOut message
-//   to the current RVH.
-// - Once the onunload handler is finished, a SwapOut_ACK message is sent to
-//   the ResourceDispatcherHost, who unpauses the response.  Data is then sent
-//   to the pending RVH.
+//   download, the CrossSiteResourceHandler checks whether a process swap is
+//   needed (either because CrossSiteRequestManager has state for it or because
+//   a transfer was needed for a redirect).
+// - If a swap is needed, CrossSiteResourceHandler pauses the response to first
+//   run the old page's unload handler.  It does this by asynchronously calling
+//   the OnCrossSiteResponse method of RenderViewHostManager on the UI thread,
+//   which sends a SwapOut message to the current RVH.
+// - Once the unload handler is finished, RVHM::SwappedOut checks if a transfer
+//   to a new process is needed, based on the stored pending_nav_params_.

[Matt Perry, 2013/06/18 22:58:51]

This comment confuses me. In the previous bullet point, it talks about whether a
swap is needed. Is "transfer to a new process" here referring to the same swap?

[Charlie Reis, 2013/06/19 00:17:01]

Ah.  A swap is needed if either we're doing a transfer or we knew it was a
cross-process navigation from the outset (e.g., cross-site URL in the omnibox,
etc).  That corresponds to the should_transfer and swap_needed variables in
CrossSiteResourceHandler::OnResponseStarted.

I clarified it a bit in this bullet, and I also made it clearer that the "if a
swap is needed" phrase is explained in the previous bullet.

+//   - If not, it just tells the ResourceDispatcherHost to resume the response
+//     to its current RenderViewHost.
+//   - If so, it cancels the current pending RenderViewHost and sets up a new
+//     navigation using RequestTransferURL.  When the transferred request
+//     arrives in the ResourceDispatcherHost, we transfer the response and
+//     resume it.
 // - The pending renderer sends a FrameNavigate message that invokes the
 //   DidNavigate method.  This replaces the current RVH with the
 //   pending RVH.