Merge CrossSiteResourceHandler and NavigationResourceThrottle

content/browser/frame_host/navigation_handle_impl.h

Index: content/browser/frame_host/navigation_handle_impl.h
diff --git a/content/browser/frame_host/navigation_handle_impl.h b/content/browser/frame_host/navigation_handle_impl.h
index 681e2b4909b3cb943e93e7cc09fd2ec553ed44c3..03e81267663c224e9f299a6f686f9ba3e56b92be 100644
--- a/content/browser/frame_host/navigation_handle_impl.h
+++ b/content/browser/frame_host/navigation_handle_impl.h
@@ -16,6 +16,7 @@
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/global_request_id.h"
 #include "content/public/browser/navigation_data.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/ssl_status.h"
@@ -28,6 +29,7 @@ namespace content {
 
 class NavigatorDelegate;
 class ResourceRequestBodyImpl;
+class SiteInstance;
 
 // This class keeps track of a single navigation. It is created upon receipt of
 // a DidStartProvisionalLoad IPC in a RenderFrameHost. The RenderFrameHost owns
@@ -151,12 +153,6 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
     is_transferring_ = is_transferring;
   }
 
-  // Updates the RenderFrameHost that is about to commit the navigation. This
-  // is used during transfer navigations.
-  void set_render_frame_host(RenderFrameHostImpl* render_frame_host) {
-    render_frame_host_ = render_frame_host;
-  }
-
   // Returns the POST body associated with this navigation.  This will be
   // null for GET and/or other non-POST requests (or if a response to a POST
   // request was a redirect that changed the method to GET - for example 302).
@@ -198,11 +194,18 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
   // NavigationHandle will not call |callback| with a result of DEFER.
   // If the result is PROCEED, then 'ReadyToCommitNavigation' will be called
   // with |render_frame_host| and |response_headers| just before calling
-  // |callback|.
+  // |callback|. Should a transfer navigation happen, |transfer_callback| will
+  // be run on the IO thread.
+  // PlzNavigate: transfer navigations are not possible.
   void WillProcessResponse(
       RenderFrameHostImpl* render_frame_host,
       scoped_refptr<net::HttpResponseHeaders> response_headers,
       const SSLStatus& ssl_status,
+      const GlobalRequestID& request_id,
+      bool should_replace_current_entry,
+      bool is_download,
+      bool is_stream,
+      const base::Closure& transfer_callback,
       const ThrottleChecksFinishedCallback& callback);
 
   // Returns the FrameTreeNode this navigation is happening in.
@@ -229,6 +232,15 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
 
   SSLStatus ssl_status() { return ssl_status_; }
 
+  const GlobalRequestID& request_id() const { return request_id_; }

[Charlie Reis, 2016/09/16 21:19:26]

Please comment about the timeline that this is valid.  How soon after the
request is created is it set?  Is it always set, or only for transfers?  (I'm
talking with Rohit about a case that might want to use this.)

[clamy, 2016/09/20 15:57:22]

Done. Added a DCHECK as well. As explained in the todo, this could potentially
be initialized earlier (though there may be some issues in PlzNavigate).

+
+  // Called when the navigation is transferred to a SiteInstance.
+  void TransferToSiteInstance(SiteInstance* site_instance);
+
+  // Called when a RenderFrameHost has been created for the SiteInstance the
+  // navigation is transferred to.
+  void TransferToRenderFrameHost(RenderFrameHostImpl* render_frame_host);
+
  private:
   friend class NavigationHandleImplTest;
 
@@ -259,6 +271,15 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
   NavigationThrottle::ThrottleCheckResult CheckWillRedirectRequest();
   NavigationThrottle::ThrottleCheckResult CheckWillProcessResponse();
 
+  // Called when WillProcessResponse checks are done, to find the final
+  // RenderFrameHost for the navigation. Returns true if the navigation should

[Charlie Reis, 2016/09/16 21:19:26]

Can you mention a case when it would return false?  I'm not sure I understand
why this would abort at first glance.  (That sounds more like something
MaybeTransferAndProceed would do.)

[clamy, 2016/09/20 15:57:22]

Updated the comment following the naming change suggested in the .cc file.

+  // proceed.
+  bool FindFinalRenderFrameHost();
+
+  // Check whether the navigation should be transferred. Returns false if the
+  // transfer attempt results in the destruction of this NavigationHandle.
+  bool MaybeTransferAndProceed();
+
   // Helper function to run and reset the |complete_callback_|. This marks the
   // end of a round of NavigationThrottleChecks.
   void RunCompleteCallback(NavigationThrottle::ThrottleCheckResult result);
@@ -284,6 +305,10 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
   bool was_redirected_;
   scoped_refptr<net::HttpResponseHeaders> response_headers_;
 
+  // The original url of the navigation. This may differ from |url_| if the
+  // navigation encounters redirects.
+  const GURL original_url_;
+
   // The HTTP method used for the navigation.
   std::string method_;
 
@@ -325,6 +350,24 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
 
   SSLStatus ssl_status_;
 
+  // The id of the URLRequest tied to this navigation.
+  GlobalRequestID request_id_;
+
+  // Whether the current NavigationEntry should be replaced upon commit.
+  bool should_replace_current_entry_;

[Charlie Reis, 2016/09/16 21:19:26]

I'm curious, why did these become necessary in the new approach?

[clamy, 2016/09/20 15:57:22]

Most of them are data that was contained in the CrossSiteTransferringRequest.
Since I'm removing this class, I need to include them in the NavigationHandle,
which now plays a role very similar to the CrossSiteTransferringRequest.

is_download_ and is_stream_ are needed to check whether we should attempt to
transfer or not.

[Charlie Reis, 2016/09/21 03:31:47]

CrossSiteTransferringRequest only had the GlobalRequestID.  I was curious about
should_replace_current_entry_ and redirect_chain_, but it looks like those used
to be posted from CrossSiteResourceHandler to RFHM::OnCrossSiteResponse, and now
we pass them in from here.  Seems reasonable.

+
+  // The chain of redirects.
+  std::vector<GURL> redirect_chain_;
+
+  // A callback to run on the IO thread if the navigation transfers.
+  base::Closure transfer_callback_;
+
+  // Whether the navigation ended up being a download or a stream.
+  bool is_download_;
+  bool is_stream_;
+
+  base::WeakPtrFactory<NavigationHandleImpl> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NavigationHandleImpl);
 };