PlzNavigate: Create the speculative renderer earlier.

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 aff92459639e88b0a6b77752b4de2b3ae1ea43d8..eb44b29b6e0d0f2146ab1d302bfb022cb12700c1 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -600,8 +600,9 @@ void NavigatorImpl::OnBeforeUnloadACK(FrameTreeNode* frame_tree_node,
   DCHECK_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
             navigation_request->state());
 
+  // If the navigation is allowed to proceed, send the request to the IO thread.
   if (proceed)
-    BeginNavigation(frame_tree_node);
+    navigation_request->BeginNavigation();
   else
     CancelNavigation(frame_tree_node);
 }
@@ -655,7 +656,13 @@ void NavigatorImpl::OnBeginNavigation(
     navigation_data_.reset();
   }
 
-  BeginNavigation(frame_tree_node);
+  if (frame_tree_node->navigation_request()->BeginNavigation()) {
+    // If the request was really sent to the IO thread, notify the
+    // RenderFrameHostManager so it can speculatively create a RenderFrameHost
+    // (and potentially a new renderer process) in parallel.
+    frame_tree_node->render_manager()->BeginNavigation(
+        *frame_tree_node->navigation_request());
+  }
 }
 
 // PlzNavigate
@@ -805,40 +812,28 @@ void NavigatorImpl::RequestNavigation(
   DCHECK(frame_tree_node);
   FrameMsg_Navigate_Type::Value navigation_type =
       GetNavigationType(controller_->GetBrowserContext(), entry, reload_type);
-  scoped_ptr<NavigationRequest> navigation_request =
+  frame_tree_node->SetNavigationRequest(
       NavigationRequest::CreateBrowserInitiated(frame_tree_node, entry,
                                                 navigation_type,
-                                                navigation_start, controller_);
-  frame_tree_node->SetNavigationRequest(navigation_request.Pass());
-  frame_tree_node->navigation_request()->SetWaitingForRendererResponse();
+                                                navigation_start, controller_));
+  NavigationRequest* navigation_request = frame_tree_node->navigation_request();
 
-  // Have the current renderer execute its beforeUnload event if needed. If it
-  // is not needed (eg. the renderer is not live), BeginNavigation should get
-  // called. If the navigation is synchronous and same-site, then it can be sent
-  // directly to the renderer (currently this is the case for navigations that
-  // do not make network requests such as data urls or Javascript urls).
+  // Have the current renderer execute its beforeunload event if needed.
   if (NavigationRequest::ShouldMakeNetworkRequest(

[clamy, 2015/06/09 10:02:01]

I find the new ordering of comments not clearer. I think having the paragraph
summary was simpler. This will be even more the case if you move the call to
RFHM inside FrameTreeNode.

[carlosk, 2015/06/09 15:33:51]

Reverted to the previous comment with slight adaptations.

-          frame_tree_node->navigation_request()->common_params().url)) {
+          navigation_request->common_params().url)) {
+    // If this navigation is not synchronous dispatch the beforeunload request.
+    navigation_request->SetWaitingForRendererResponse();
     frame_tree_node->current_frame_host()->DispatchBeforeUnload(true);
-  } else {
-    BeginNavigation(frame_tree_node);
-  }
-}
-
-void NavigatorImpl::BeginNavigation(FrameTreeNode* frame_tree_node) {
-  NavigationRequest* navigation_request = frame_tree_node->navigation_request();
-
-  // A browser-initiated navigation could have been cancelled while it was
-  // waiting for the BeforeUnload event to execute.
-  if (!navigation_request)
-    return;
-
-  // Start the request.
-   if (navigation_request->BeginNavigation()) {
-    // If the request was sent to the IO thread, notify the
-    // RenderFrameHostManager so it can speculatively create a RenderFrameHost
-    // (and potentially a new renderer process) in parallel.
+    // Note: At this point if beforeunload was deemed not needed (i.e. there
+    // was no live renderer) then the request to the IO thread has already been
+    // sent.
     frame_tree_node->render_manager()->BeginNavigation(*navigation_request);
+  } else {
+    // If the navigation is synchronous and same-site, then the current renderer
+    // will be reused and the NavigationRequest will just commit the navigation.
+    // This is currently the case for navigations that do not make network
+    // requests such as data URLs or Javascript URLs.
+    navigation_request->BeginNavigation();
   }
 }