PlzNavigate: implement CommitNavigation on the browser side

content/browser/frame_host/navigator_impl.cc

Index: content/browser/frame_host/navigator_impl.cc
diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
index 7bde6cd9a54537e86539b1e3179fd99286986710..2548b98544f5ac73dc13bba6e53a306a866a7432 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -11,6 +11,7 @@
 #include "content/browser/frame_host/navigation_before_commit_info.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
+#include "content/browser/frame_host/navigation_parameters.h"
 #include "content/browser/frame_host/navigator_delegate.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
@@ -71,40 +72,21 @@ RenderFrameHostManager* GetRenderManager(RenderFrameHostImpl* rfh) {
   return rfh->frame_tree_node()->frame_tree()->root()->render_manager();
 }
 
-}  // namespace
-
-
-NavigatorImpl::NavigatorImpl(
-    NavigationControllerImpl* navigation_controller,
-    NavigatorDelegate* delegate)
-    : controller_(navigation_controller),
-      delegate_(delegate) {
-}
-
-// static.
-void NavigatorImpl::MakeNavigateParams(
+void MakeNavigateParams(
     const NavigationEntryImpl& entry,
-    const NavigationControllerImpl& controller,
+    NavigationControllerImpl* controller,
     NavigationController::ReloadType reload_type,
     base::TimeTicks navigation_start,
     FrameMsg_Navigate_Params* params) {
   params->page_id = entry.GetPageID();
   params->should_clear_history_list = entry.should_clear_history_list();
   params->should_replace_current_entry = entry.should_replace_entry();
-  if (entry.should_clear_history_list()) {
-    // Set the history list related parameters to the same values a
-    // NavigationController would return before its first navigation. This will
-    // fully clear the RenderView's view of the session history.
-    params->pending_history_list_offset = -1;
-    params->current_history_list_offset = -1;
-    params->current_history_list_length = 0;
-  } else {
-    params->pending_history_list_offset = controller.GetIndexOfEntry(&entry);
-    params->current_history_list_offset =
-        controller.GetLastCommittedEntryIndex();
-    params->current_history_list_length = controller.GetEntryCount();
-  }
   params->url = entry.GetURL();
+  controller->FillHistoryParametersForNavigationEntry(
+      entry,
+      &params->pending_history_list_offset,
+      &params->current_history_list_offset,
+      &params->current_history_list_length);
   if (!entry.GetBaseURLForDataURL().is_empty()) {
     params->base_url_for_data_url = entry.GetBaseURLForDataURL();
     params->history_url_for_data_url = entry.GetVirtualURL();
@@ -112,8 +94,8 @@ void NavigatorImpl::MakeNavigateParams(
   params->referrer = entry.GetReferrer();
   params->transition = entry.GetTransitionType();
   params->page_state = entry.GetPageState();
-  params->navigation_type =
-      GetNavigationType(controller.GetBrowserContext(), entry, reload_type);
+  params->navigation_type = GetNavigationType(
+      controller->GetBrowserContext(), entry, reload_type);
   // This is used by the old performance infrastructure to set up DocumentState
   // associated with the RenderView.
   // TODO(ppi): make it go away.
@@ -147,6 +129,16 @@ void NavigatorImpl::MakeNavigateParams(
   params->browser_navigation_start = navigation_start;
 }
 
+}  // namespace
+
+
+NavigatorImpl::NavigatorImpl(
+    NavigationControllerImpl* navigation_controller,
+    NavigatorDelegate* delegate)
+    : controller_(navigation_controller),
+      delegate_(delegate) {
+}
+
 NavigationController* NavigatorImpl::GetController() {
   return controller_;
 }
@@ -348,11 +340,6 @@ bool NavigatorImpl::NavigateToEntry(
   // TODO(ppi): make this go away.
   current_load_start_ = base::TimeTicks::Now();
 
-  // Create the navigation parameters.
-  FrameMsg_Navigate_Params navigate_params;
-  MakeNavigateParams(
-      entry, *controller_, reload_type, navigation_start, &navigate_params);
-
   RenderFrameHostManager* manager =
       render_frame_host->frame_tree_node()->render_manager();
 
@@ -361,7 +348,14 @@ bool NavigatorImpl::NavigateToEntry(
   // node.
   if (CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation)) {
-    return manager->RequestNavigation(entry, navigate_params);
+    scoped_ptr<NavigationParameters> navigation_parameters(
+        new NavigationParameters(
+            entry,
+            controller_,
+            current_load_start_,
+            GetNavigationType(
+                controller_->GetBrowserContext(), entry, reload_type)));
+    return manager->RequestNavigation(navigation_parameters.Pass());
   }
 
   RenderFrameHostImpl* dest_render_frame_host = manager->Navigate(entry);
@@ -380,6 +374,11 @@ bool NavigatorImpl::NavigateToEntry(
   if (delegate_)
     delegate_->AboutToNavigateRenderFrame(dest_render_frame_host);
 
+  // Create the navigation parameters.
+  FrameMsg_Navigate_Params navigate_params;
+  MakeNavigateParams(
+      entry, controller_, reload_type, navigation_start, &navigate_params);
+
   // Navigate in the desired RenderFrameHost.
   // We can skip this step in the rare case that this is a transfer navigation
   // which began in the chosen RenderFrameHost, since the request has already
@@ -664,11 +663,21 @@ void NavigatorImpl::RequestTransferURL(
 
 void NavigatorImpl::CommitNavigation(
     RenderFrameHostImpl* render_frame_host,
-    const NavigationBeforeCommitInfo& info) {
+    const FrameMsg_CommitNavigation_Params& commit_navigation_params) {
   CheckWebUIRendererDoesNotDisplayNormalURL(
-      render_frame_host, info.navigation_url);
-  // TODO(clamy): the render_frame_host should now send a commit IPC to the
-  // renderer.
+      render_frame_host, commit_navigation_params.url);
+  render_frame_host->CommitNavigation(commit_navigation_params);
+}
+
+NavigationParameters* NavigatorImpl::MakeNavigationParametersForTest(

[(Do not use) nasko, 2014/09/05 17:04:48]

I know this is for testing, but it seems that it will be good for this to return
a scoped_ptr, so the test doesn't have to explicitly delete it. We shouldn't be
leaking memory even in tests, otherwise leak detection can only be done in real
Chrome instance, which is not ideal.

[clamy, 2014/09/08 18:37:12]

Done.

+    const NavigationEntryImpl& entry,
+    NavigationController::ReloadType reload_type) {
+  FrameMsg_Navigate_Type::Value navigation_type = GetNavigationType(
+      controller_->GetBrowserContext(),
+      entry,
+      reload_type);
+  return new NavigationParameters(
+      entry, controller_, base::TimeTicks::Now(), navigation_type);
 }
 
 void NavigatorImpl::CheckWebUIRendererDoesNotDisplayNormalURL(