PlzNavigate: implement CommitNavigation on the browser side

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index b97c76bc50e2f4dab3a65b55e0ec77ee8eafc6d2..2772a758b92d430a75703c551b11961554551db2 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -30,6 +30,7 @@
 #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/navigation_params.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_service.h"
@@ -45,36 +46,49 @@
 
 namespace content {
 
-namespace {
+// PlzNavigate
+// Returns the net load flags to use based on the navigation type.
+int LoadFlagFromNavigationType(FrameMsg_Navigate_Type::Value navigation_type) {

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

Where is this code from?  (In other words, how does this work in the
non-PlzNavigate case?)

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

This code is here to handle the case where we don't have a renderer. Currently
the caching policy is decided by blink in various places (in particular
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)
and then translated to a load flag in WebURLLoaderImpl
(https://code.google.com/p/chromium/codesearch#chromium/src/content/child/web_...).
I have plans to unify it in the future. Added a TODO to reflect that.

+  int load_flags = net::LOAD_NORMAL;
+  switch (navigation_type) {
+    case FrameMsg_Navigate_Type::RELOAD:
+    case FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL:
+      load_flags |= net::LOAD_VALIDATE_CACHE;
+      break;
+    case FrameMsg_Navigate_Type::RELOAD_IGNORING_CACHE:
+      load_flags |= net::LOAD_BYPASS_CACHE;
+      break;
+    case FrameMsg_Navigate_Type::RESTORE:
+      load_flags |= net::LOAD_PREFERRING_CACHE;
+      break;
+    case FrameMsg_Navigate_Type::RESTORE_WITH_POST:
+      load_flags |= net::LOAD_ONLY_FROM_CACHE;
+      break;
+    case FrameMsg_Navigate_Type::NORMAL:
+    default:
+      break;
+  }
+  return load_flags;
+}
 
 // PlzNavigate
-// Simulates a renderer response to a navigation request when there is no live
-// renderer.
-FrameHostMsg_BeginNavigation_Params BeginNavigationFromNavigate(
-    const FrameMsg_Navigate_Params& navigate_params) {
+// Generates a default FrameHostMsg_BeginNavigation_Params to be used when there
+// is no live renderer.
+FrameHostMsg_BeginNavigation_Params MakeDefaultBeginNavigation(
+    const RequestNavigationParams& request_params,
+    FrameMsg_Navigate_Type::Value navigation_type) {
   FrameHostMsg_BeginNavigation_Params begin_navigation_params;
-  begin_navigation_params.method = navigate_params.is_post ? "POST" : "GET";
-  begin_navigation_params.url = navigate_params.url;
-  begin_navigation_params.referrer =
-      Referrer(navigate_params.referrer.url, navigate_params.referrer.policy);
-
-  // TODO(clamy): This should be modified to take into account caching policy
-  // requirements (eg for POST reloads).
-  begin_navigation_params.load_flags = net::LOAD_NORMAL;
+  begin_navigation_params.method = request_params.is_post ? "POST" : "GET";
+  begin_navigation_params.load_flags =
+      LoadFlagFromNavigationType(navigation_type);
 
   // TODO(clamy): Post data from the browser should be put in the request body.
+  // Headers should be filled in as well.
 
   begin_navigation_params.has_user_gesture = false;
-  begin_navigation_params.transition_type = navigate_params.transition;
-  begin_navigation_params.should_replace_current_entry =
-      navigate_params.should_replace_current_entry;
-  begin_navigation_params.allow_download =
-      navigate_params.allow_download;
   return begin_navigation_params;
 }
 
-}  // namespace
-
 bool RenderFrameHostManager::ClearRFHsPendingShutdown(FrameTreeNode* node) {
   node->render_manager()->pending_delete_hosts_.clear();
   return true;
@@ -566,34 +580,47 @@ void RenderFrameHostManager::ResetProxyHosts() {
 
 // PlzNavigate
 bool RenderFrameHostManager::RequestNavigation(
-    const NavigationEntryImpl& entry,
-    const FrameMsg_Navigate_Params& navigate_params) {
+    scoped_ptr<CoreNavigationParams> core_params,
+    const RequestNavigationParams& request_params,
+    scoped_ptr<CommitNavigationParams> commit_params) {
   CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
+  DCHECK(commit_params.get() && core_params.get());
+  core_navigation_params_ = core_params.Pass();
+  commit_navigation_params_ = commit_params.Pass();
   if (render_frame_host_->IsRenderFrameLive()) {
     // TODO(clamy): send a RequestNavigation IPC.
     return true;
   }
 
   // The navigation request is sent directly to the IO thread.
-  OnBeginNavigation(BeginNavigationFromNavigate(navigate_params));
+  OnBeginNavigation(
+      MakeDefaultBeginNavigation(request_params,
+                                 core_navigation_params_->navigation_type),
+      *core_navigation_params_);
   return true;
 }
 
 // PlzNavigate
 void RenderFrameHostManager::OnBeginNavigation(
-    const FrameHostMsg_BeginNavigation_Params& params) {
+    const FrameHostMsg_BeginNavigation_Params& params,
+    const CoreNavigationParams& core_params) {
   CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
+  DCHECK(core_navigation_params_.get());
+  core_navigation_params_->referrer = core_params.referrer;
   // TODO(clamy): Check if navigations are blocked and if so, return
   // immediately.
-  NavigationRequestInfo info(params);
+  NavigationRequestInfo info(*core_navigation_params_, params);
 
-  info.first_party_for_cookies = frame_tree_node_->IsMainFrame() ?
-      params.url : frame_tree_node_->frame_tree()->root()->current_url();
+  info.first_party_for_cookies =
+      frame_tree_node_->IsMainFrame()
+          ? core_navigation_params_->url
+          : frame_tree_node_->frame_tree()->root()->current_url();
   info.is_main_frame = frame_tree_node_->IsMainFrame();
   info.parent_is_main_frame = !frame_tree_node_->parent() ?
       false : frame_tree_node_->parent()->IsMainFrame();
+  info.allow_download = core_navigation_params_->allow_download;
 
   // TODO(clamy): Check if the current RFH should be initialized (in case it has
   // crashed) not to display a sad tab while navigating.
@@ -622,6 +649,8 @@ void RenderFrameHostManager::CommitNavigation(
     return;
   }
 
+  DCHECK(commit_navigation_params_.get());
+  DCHECK(core_navigation_params_.get());
   // Pick the right RenderFrameHost to commit the navigation.
   SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
   // TODO(clamy): Replace the default values by the right ones. This may require
@@ -629,7 +658,7 @@ void RenderFrameHostManager::CommitNavigation(
   SiteInstance* new_instance = GetSiteInstanceForNavigation(
       info.navigation_url,
       NULL,
-      navigation_request_->info().navigation_params.transition_type,
+      core_navigation_params_->transition,
       false,
       false);
   DCHECK(!pending_render_frame_host_.get());
@@ -661,8 +690,10 @@ void RenderFrameHostManager::CommitNavigation(
     }
   }
 
-  frame_tree_node_->navigator()->CommitNavigation(
-      render_frame_host_.get(), info);
+  frame_tree_node_->navigator()->CommitNavigation(render_frame_host_.get(),
+                                                  info.stream_url,
+                                                  *core_navigation_params_,

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

These are scoped_ptrs.  Shouldn't we be passing ownership of them and clearing
them out?

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

They are now members of NavigationRequest.

+                                                  *commit_navigation_params_);
 }
 
 void RenderFrameHostManager::Observe(