PlzNavigate: Move the navigation logic to NavigatorImpl

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 683b2452fb60b966f1def817cffdadebd10e378e..f22ff36abbedfe3a78bc9291a40b5d85fe8ddc45 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -12,13 +12,13 @@
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
+#include "content/browser/frame_host/navigation_request_info.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"
 #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/frame_messages.h"
 #include "content/common/navigation_params.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
@@ -34,6 +34,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
+#include "net/base/load_flags.h"
 
 namespace content {
 
@@ -65,6 +66,50 @@ FrameMsg_Navigate_Type::Value GetNavigationType(
   return FrameMsg_Navigate_Type::NORMAL;
 }
 
+// PlzNavigate
+// Returns the net load flags to use based on the navigation type.
+// TODO(clamy): unify the code with what is happening on the renderer side.
+int LoadFlagFromNavigationType(FrameMsg_Navigate_Type::Value navigation_type) {
+  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
+// 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 = 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;
+  return begin_navigation_params;
+}
+
 RenderFrameHostManager* GetRenderManager(RenderFrameHostImpl* rfh) {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kSitePerProcess))
@@ -308,28 +353,13 @@ bool NavigatorImpl::NavigateToEntry(
   RenderFrameHostManager* manager =
       render_frame_host->frame_tree_node()->render_manager();
 
-  // PlzNavigate: the RenderFrameHosts are no longer asked to navigate. Instead
-  // the RenderFrameHostManager handles the navigation requests for that frame
-  // node.
+  // PlzNavigate: the RenderFrameHosts are no longer asked to navigate.
   if (CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation)) {
-    FrameMsg_Navigate_Type::Value navigation_type =
-        GetNavigationType(controller_->GetBrowserContext(), entry, reload_type);
-    scoped_ptr<NavigationRequest> navigation_request(new NavigationRequest(
-        render_frame_host->frame_tree_node()->frame_tree_node_id(),
-        CommonNavigationParams(entry.GetURL(),
-                               entry.GetReferrer(),
-                               entry.GetTransitionType(),
-                               navigation_type,
-                               !entry.IsViewSourceMode()),
-        CommitNavigationParams(entry.GetPageState(),
-                               entry.GetIsOverridingUserAgent(),
-                               navigation_start)));
-    RequestNavigationParams request_params(entry.GetHasPostData(),
-                                           entry.extra_headers(),
-                                           entry.GetBrowserInitiatedPostData());
-    return manager->RequestNavigation(navigation_request.Pass(),
-                                      request_params);
+    return RequestNavigation(render_frame_host->frame_tree_node(),
+                             entry,
+                             reload_type,
+                             navigation_start);
   }
 
   RenderFrameHostImpl* dest_render_frame_host = manager->Navigate(entry);
@@ -407,6 +437,15 @@ bool NavigatorImpl::NavigateToPendingEntry(
 void NavigatorImpl::DidNavigate(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& input_params) {
+  // PlzNavigate
+  // The navigation request has been committed so the browser process doesn't
+  // need to care about it anymore.
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation)) {
+    navigation_request_map_.erase(
+        render_frame_host->frame_tree_node()->frame_tree_node_id());
+  }
+
   FrameHostMsg_DidCommitProvisionalLoad_Params params(input_params);
   FrameTree* frame_tree = render_frame_host->frame_tree_node()->frame_tree();
   bool use_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -639,16 +678,88 @@ void NavigatorImpl::RequestTransferURL(
     delegate_->RequestOpenURL(render_frame_host, params);
 }
 
+// PlzNavigate
+void NavigatorImpl::OnBeginNavigation(
+    FrameTreeNode* frame_tree_node,
+    const FrameHostMsg_BeginNavigation_Params& params,
+    const CommonNavigationParams& common_params) {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  DCHECK(frame_tree_node);
+
+  // TODO(clamy): In case of a renderer initiated navigation create a new
+  // NavigationRequest.
+  NavigationRequestMap::iterator it = navigation_request_map_.find(
+      frame_tree_node->frame_tree_node_id());
+  DCHECK(it != navigation_request_map_.end());
+  NavigationRequest* navigation_request = it->second.get();

[carlosk, 2014/10/01 18:31:09]

In the case of a hijacked renderer process it could send bogus
FrameHostMsg_BeginNavigation (if there's no navigation going on) that would
cause a crash/undefined behavior here because the find wouldn't find anything.
Is this something we normally worry about?

[clamy, 2014/10/01 22:30:46]

Yes, but this is actually covered by the TODO above. When we receive a
FrameHostMsg_BeginNavigation we should create a NavigationRequest if we don't
have one. I intend to change that in a follow up CL.

+
+  // Update the referrer with the one received from the renderer.
+  navigation_request->common_params().referrer = common_params.referrer;
+
+  scoped_ptr<NavigationRequestInfo> info(new NavigationRequestInfo(params));
+
+  info->first_party_for_cookies =
+      frame_tree_node->IsMainFrame()
+          ? navigation_request->common_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();
+
+  // TODO(clamy): Inform the RenderFrameHostManager that a navigation is about
+  // to begin, so that it can speculatively spawn a new renderer if needed.
+
+  navigation_request->BeginNavigation(info.Pass(), params.request_body);
+}
+
+// PlzNavigate
 void NavigatorImpl::CommitNavigation(
-    RenderFrameHostImpl* render_frame_host,
-    const GURL& stream_url,
-    const CommonNavigationParams& common_params,
-    const CommitNavigationParams& commit_params) {
-  CheckWebUIRendererDoesNotDisplayNormalURL(render_frame_host,
-                                            common_params.url);
-  render_frame_host->CommitNavigation(stream_url, common_params, commit_params);
+    FrameTreeNode* frame_tree_node,
+    const NavigationBeforeCommitInfo& info) {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  NavigationRequestMap::iterator it = navigation_request_map_.find(
+      frame_tree_node->frame_tree_node_id());
+  DCHECK(it != navigation_request_map_.end());
+  NavigationRequest* navigation_request = it->second.get();
+
+  // Ignores navigation commits if the request ID doesn't match the current
+  // active request.
+  if (navigation_request->navigation_request_id() !=
+          info.navigation_request_id) {
+    return;
+  }
+
+  // Update the navigation url.
+  navigation_request->common_params().url = info.navigation_url;
+
+  // Select an appropriate renderer to commit the navigation.
+  RenderFrameHostImpl* render_frame_host =
+      frame_tree_node->render_manager()->GetFrameHostForNavigation(
+          info.navigation_url, navigation_request->common_params().transition);
+  CheckWebUIRendererDoesNotDisplayNormalURL(
+      render_frame_host, navigation_request->common_params().url);
+
+  render_frame_host->CommitNavigation(info.stream_url,
+                                      navigation_request->common_params(),
+                                      navigation_request->commit_params());
+}
+
+// PlzNavigate
+void NavigatorImpl::CancelNavigation(FrameTreeNode* frame_tree_node) {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  NavigationRequestMap::iterator it = navigation_request_map_.find(
+      frame_tree_node->frame_tree_node_id());
+  if (it == navigation_request_map_.end())
+    return;
+  it->second->CancelNavigation();
+  navigation_request_map_.erase(frame_tree_node->frame_tree_node_id());
 }
 
+NavigatorImpl::~NavigatorImpl() {}
+
 void NavigatorImpl::CheckWebUIRendererDoesNotDisplayNormalURL(
     RenderFrameHostImpl* render_frame_host,
     const GURL& url) {
@@ -665,4 +776,56 @@ void NavigatorImpl::CheckWebUIRendererDoesNotDisplayNormalURL(
   }
 }
 
+// PlzNavigate
+bool NavigatorImpl::RequestNavigation(
+    FrameTreeNode* frame_tree_node,
+    const NavigationEntryImpl& entry,
+    NavigationController::ReloadType reload_type,
+    base::TimeTicks navigation_start) {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  DCHECK(frame_tree_node);
+  int64 frame_tree_node_id = frame_tree_node->frame_tree_node_id();
+  FrameMsg_Navigate_Type::Value navigation_type =
+      GetNavigationType(controller_->GetBrowserContext(), entry, reload_type);
+  linked_ptr<NavigationRequest> navigation_request(new NavigationRequest(
+      frame_tree_node_id,
+      CommonNavigationParams(entry.GetURL(),
+                             entry.GetReferrer(),
+                             entry.GetTransitionType(),
+                             navigation_type,
+                             !entry.IsViewSourceMode()),
+      CommitNavigationParams(entry.GetPageState(),
+                             entry.GetIsOverridingUserAgent(),
+                             navigation_start)));
+  RequestNavigationParams request_params(entry.GetHasPostData(),
+                                         entry.extra_headers(),
+                                         entry.GetBrowserInitiatedPostData());
+  // TODO(clamy): Check if navigations are blocked and if so store the
+  // parameters.
+
+  // If there is an ongoing request it must be canceled.
+  NavigationRequestMap::iterator it = navigation_request_map_.find(
+      frame_tree_node_id);
+  if (it != navigation_request_map_.end())
+    it->second->CancelNavigation();

[nasko, 2014/10/01 15:59:49]

Shouldn't we also erase the element from the map?

[clamy, 2014/10/01 17:10:43]

We are replacing it just afterwards so it should be fine, I think.

[nasko, 2014/10/01 18:20:18]

AFAIU, insert will not overwrite existing entries. It might be safer to delete
or check the return value from insert.

[clamy, 2014/10/01 22:30:46]

Indeed it does not. But I am now using ScopedPtrHashMap::set which does.

[nasko, 2014/10/02 00:09:39]

Acknowledged.

+
+  navigation_request_map_.insert(
+      std::pair<int64, linked_ptr<NavigationRequest> >(
+          frame_tree_node_id, navigation_request));

[carlosk, 2014/10/01 18:31:10]

I was also looking into this matter of the entry already existing in the map. It
seems that for some implementations of hash_map.insert -- and IDK precisely
which one we use even after finding it's "absorbed" from the
compiler-standard-one in hash_tables.h -- it *won't* replace an element if it
already exists.

My suggestion is, if the element does exist, to use the returned iterator to
replace it->second with the new value. I think that works (again depending on
our implementation of hash_map)...

+
+  if (frame_tree_node->current_frame_host()->IsRenderFrameLive()) {
+    // TODO(clamy): send a RequestNavigation IPC.
+    return true;
+  }
+
+  // The navigation request is sent directly to the IO thread.
+  OnBeginNavigation(
+      frame_tree_node,
+      MakeDefaultBeginNavigation(
+          request_params, navigation_request->common_params().navigation_type),
+      navigation_request->common_params());
+  return true;
+}
+
 }  // namespace content