PlzNavigate: implement CommitNavigation on the browser side

content/browser/frame_host/render_frame_host_manager.h

 // 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.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_
 #define CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 24 matching lines @@
 class RenderFrameHost;
 class RenderFrameHostImpl;
 class RenderFrameHostManagerTest;
 class RenderFrameProxyHost;
 class RenderViewHost;
 class RenderViewHostImpl;
 class RenderWidgetHostDelegate;
 class RenderWidgetHostView;
 class TestWebContents;
 class WebUIImpl;
+struct CommitNavigationParams;
+struct CoreNavigationParams;
 struct NavigationBeforeCommitInfo;
+struct RequestNavigationParams;
 
 // Manages RenderFrameHosts for a FrameTreeNode.  This class acts as a state
 // machine to make cross-process navigations in a frame possible.
 class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
  public:
   // Functions implemented by our owner that we need.
   //
   // TODO(brettw) Clean this up! These are all the functions in WebContentsImpl
   // that are required to run this class. The design should probably be better
   // such that these are more clear.
@@ skipping 252 matching lines @@
   // of WebContentsImpl.
   void ResetProxyHosts();
 
   // Returns the routing id for a RenderFrameHost or RenderFrameHostProxy
   // that has the given SiteInstance and is associated with this
   // RenderFrameHostManager. Returns MSG_ROUTING_NONE if none is found.
   int GetRoutingIdForSiteInstance(SiteInstance* site_instance);
 
   // PlzNavigate: sends a RequestNavigation IPC to the renderer to ask it to
   // navigate. If no live renderer is present, then the navigation request will
-  // be sent directly to the ResourceDispatcherHost.
-  bool RequestNavigation(const NavigationEntryImpl& entry,
-                         const FrameMsg_Navigate_Params& navigate_params);
+  // be sent directly to the ResourceDispatcherHost. Takes ownership of
+  // |core_params| and |commit_params| to send them to the appropriate renderer
+  // when the navigation is ready to commit.
+  bool RequestNavigation(scoped_ptr<CoreNavigationParams> core_params,
+                         const RequestNavigationParams& request_params,
+                         scoped_ptr<CommitNavigationParams> commit_params);
 
   // PlzNavigate: Used to start a navigation. OnBeginNavigation is called
   // directly by RequestNavigation when there is no live renderer. Otherwise, it
   // is called following a BeginNavigation IPC from the renderer (which in
   // browser-initiated navigation also happens after RequestNavigation has been
   // called).
-  void OnBeginNavigation(const FrameHostMsg_BeginNavigation_Params& params);
+  void OnBeginNavigation(const FrameHostMsg_BeginNavigation_Params& params,
+                         const CoreNavigationParams& core_params);
 
   // PlzNavigate: Called when a navigation request has received a response, to
   // select a renderer to use for the navigation.
   void CommitNavigation(const NavigationBeforeCommitInfo& info);
 
  private:
   friend class RenderFrameHostManagerTest;
   friend class TestWebContents;
 
   FRIEND_TEST_ALL_PREFIXES(CrossProcessFrameTreeBrowserTest,
@@ skipping 190 matching lines @@
   // The intersitial page currently shown if any, not own by this class
   // (the InterstitialPage is self-owned, it deletes itself when hidden).
   InterstitialPageImpl* interstitial_page_;
 
   NotificationRegistrar registrar_;
 
   // PlzNavigate: Owns a navigation request that originated in that frame until
   // it commits.
   scoped_ptr<NavigationRequest> navigation_request_;
 
+  // PlzNavigate: Holds info that should be sent to the renderer when the

[Charlie Reis, 2014/09/19 23:12:31]

Do these have the same lifetime as navigation_request_?  Should they live there
instead?  (If not, please clarify how their lifetime differs in the comment.)

[clamy, 2014/09/23 21:13:25]

I moved them inside navigation_request_. They did not have the same lifetime
initially, but it is probably better if they do.

+  // navigation is about to commit.
+  scoped_ptr<CoreNavigationParams> core_navigation_params_;
+  scoped_ptr<CommitNavigationParams> commit_navigation_params_;

[Charlie Reis, 2014/09/19 23:12:31]

(I'm still a bit nervous about storing request-specific state in RFHM, with the
risk of it being overwritten by a different navigation request.  If this data
lived with the request on the IO thread and was then passed back, then RFHM
would be stateless with respect to ResourceRequests.  The thread hop seems
problematic, though, and we probably don't want a map of requests indexed by
navigation_request_id.  Perhaps storing it here is ok for the time being.)

[clamy, 2014/09/23 21:13:25]

I think storing them in the NavigationRequest makes it simpler, especially as
davidben@ has a CL that clarifies thread hops involving the NavigationRequest.

+
   base::WeakPtrFactory<RenderFrameHostManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManager);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_