PlzNavigate: Speculatively spawns a renderer process for navigations.

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 259653353a761112b145127f7138abde8cfba0ae..342f356b6b0d022f24b4e224fd437ab3415ac2c2 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -61,6 +61,7 @@ RenderFrameHostManager::RenderFrameHostManager(
       render_view_delegate_(render_view_delegate),
       render_widget_delegate_(render_widget_delegate),
       interstitial_page_(NULL),
+      should_reuse_web_ui_(false),
       weak_factory_(this) {
   DCHECK(frame_tree_node_);
 }
@@ -69,6 +70,11 @@ RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_)
     UnsetPendingRenderFrameHost();
 
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    UnsetSpeculativeRenderFrameHost();
+  }
+
   // We should always have a current RenderFrameHost except in some tests.
   SetRenderFrameHost(scoped_ptr<RenderFrameHostImpl>());
 
@@ -288,6 +294,8 @@ void RenderFrameHostManager::OnBeforeUnloadACK(
     bool proceed,
     const base::TimeTicks& proceed_time) {
   if (for_cross_site_transition) {
+    DCHECK(!CommandLine::ForCurrentProcess()->HasSwitch(

[nasko, 2014/12/18 00:22:39]

Why not move this check up? Should we ever see OnBeforeUnloadACK in PlzNavigate
world?

[carlosk, 2014/12/19 04:27:15]

I'm not super sure but it seems it would. My indirect evidence is that 
RenderFrameHostImpl::OnBeforeUnloadACK calls this and it was there that I added
those new navigation metrics and they do work with PlzNavigate.

[clamy, 2014/12/19 13:35:57]

We will still see OnBeforeUnloadACK for tab closes.

[carlosk, 2014/12/29 16:40:16]

Acknowledged.

+        switches::kEnableBrowserSideNavigation));
     // Ignore if we're not in a cross-site navigation.
     if (!cross_navigation_pending_)
       return;
@@ -420,6 +428,30 @@ void RenderFrameHostManager::ClearNavigationTransitionData() {
 void RenderFrameHostManager::DidNavigateFrame(
     RenderFrameHostImpl* render_frame_host,
     bool was_caused_by_user_gesture) {
+  DCHECK(render_frame_host);
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    // TODO(carlosk): we have to figure out what to do with the
+    // |render_frame_host| parameter for PlzNavigate. We mus whether eliminate
+    // it or fix it's value so we can do similar checks as are currently done
+    // below against the current and pending instances.
+    // TODO(carlosk): when this method is triggered by the renderer we have to
+    // find a way to check if that navigation wasn't canceled in the meantime.
+    CommitPending();

[clamy, 2014/12/16 15:22:48]

No we shouldn't eliminate this parameter. Also this method is always triggered
by the renderer since it is called in response to receiving an IPC from it. What
we should be doing here is roughly:

if (render_frame_host == speculative_render_frame_host_.get()) {
  CommitPending();
  DCHECK(!speculative_render_frame_host_.get());
} else if (render_frame_host == render_frame_host_.get()) {
  if (!is_same_document_navigation) {
    // This is a same-site navigation driven by the browser. There should not be
a speculative RenderFrameHost.
    DCHECK(!speculative_render_frame_host_.get());
  } else {
    if (was_caused_by_user_gesture) {
      // TODO: make sure the ongoing navigation request is properly canceled
    }
  }
} else {
  // No one else should be sending us a DidNavigate in this state.
  DCHECK(false);
}

This requires introducing an additional boolean to the arguments of the function
to distinguish same-document navigations from the rest. Also you may want to
think about merging this logic with the one below.

[carlosk, 2014/12/19 04:27:15]

OK, now I understand. I hadn't realized before that this method could be invoked
by the renderer without an existing ongoing navigation.

I implemented it but chose not to mix it with the similar code below to keep the
PlzNavigate version separate and consequentially cleaner.

I left the cancellation TODO there. Is it OK to leave it for a following  CL?

[clamy, 2014/12/19 13:35:57]

Yes. I think this CL has enough in it already that we don't need to tackle that
right now.

[carlosk, 2014/12/29 16:40:16]

Acknowledged.

+    DCHECK(!speculative_render_frame_host_);
+
+    // If the renderer that needs to navigate is not live (it was just created

[clamy, 2014/12/16 15:22:48]

Why is that block here? The code in this function should be pretty close to what
is below. Any initialization work should be done in GetFrameHostForNavigation.

[carlosk, 2014/12/19 04:27:14]

Acknowledged. I didn't realize before that it was at that point that the
renderer needed to exist so that it could receive the commit IPC.

+    // or it crashed), initialize it.
+    if (!render_frame_host_->render_view_host()->IsRenderViewLive()) {
+      // Recreate the opener chain.
+      int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager(
+          render_frame_host_->GetSiteInstance());
+      InitRenderView(render_frame_host_->render_view_host(), opener_route_id,
+                     MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame());
+    }
+    return;
+  }
+
   if (!cross_navigation_pending_) {
     DCHECK(!pending_render_frame_host_);
 
@@ -579,7 +611,10 @@ void RenderFrameHostManager::DiscardUnusedFrame(
     RenderFrameProxyHost* proxy =
         new RenderFrameProxyHost(site_instance, frame_tree_node_);
     proxy_hosts_[site_instance->GetId()] = proxy;
-    render_frame_host->SwapOut(proxy);
+
+    if (!render_frame_host->is_swapped_out())
+      render_frame_host->SwapOut(proxy);
+
     if (frame_tree_node_->IsMainFrame())
       proxy->TakeFrameHostOwnership(render_frame_host.Pass());
   } else {
@@ -624,35 +659,152 @@ void RenderFrameHostManager::ResetProxyHosts() {
 }
 
 // PlzNavigate
+void RenderFrameHostManager::BeginNavigation(
+    const FrameHostMsg_BeginNavigation_Params& params,
+    const CommonNavigationParams& common_params) {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  // Cleans up any state in case there's an ongoing navigation.
+  // TODO(carlosk): remove this cleanup here once we properly cancel ongoing
+  // navigations.
+  CleanUpNavigation();
+
+  SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
+  // TODO(carlosk): Replace the default values with the right ones for
+  // dest_instance, dest_is_restore, dest_is_view_source_mode.
+  scoped_refptr<SiteInstanceImpl> new_instance =
+      static_cast<SiteInstanceImpl*>(GetSiteInstanceForNavigation(
+          common_params.url, nullptr, common_params.transition, false, false));
+
+  // Will keep the current RFH if its SiteInstance matches the new one from
+  // the navigation or if this is a subframe navigation. If --site-per-process
+  // is enabled the RFH is never kept when sites don't match.
+  // TODO(carlosk): do not swap processes for renderer initiated navigations
+  // (see crbug.com/440266).
+  if (current_instance == new_instance.get() ||
+      (!frame_tree_node_->IsMainFrame() &&
+       !CommandLine::ForCurrentProcess()->HasSwitch(
+           switches::kSitePerProcess))) {
+    // Make sure the current RFH is alive.
+    if (!render_frame_host_->render_view_host()->IsRenderViewLive()) {
+      // Recreate the opener chain.
+      int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager(
+          render_frame_host_->GetSiteInstance());
+      bool success = InitRenderView(render_frame_host_->render_view_host(),
+                                    opener_route_id, MSG_ROUTING_NONE,
+                                    frame_tree_node_->IsMainFrame());
+      DCHECK(success);
+    }
+    DCHECK(current_instance->GetProcess()->HasConnection());
+    return;
+  }
+
+  // Speculatively create a new RFH.
+  // TODO(carlosk): enable bindings check below.
+  bool success = CreateSpeculativeRenderFrameHost(
+      common_params.url, current_instance, new_instance.get(),
+      NavigationEntryImpl::kInvalidBindings);
+  DCHECK(success);
+  DCHECK(new_instance->GetProcess()->HasConnection());
+}
+
+// PlzNavigate
+bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
+    const GURL& url,
+    SiteInstance* old_instance,
+    SiteInstance* new_instance,
+    int bindings) {
+  CHECK(new_instance);
+  CHECK_NE(old_instance, new_instance);
+
+  const NavigationEntry* current_navigation_entry =
+      delegate_->GetLastCommittedNavigationEntryForRenderManager();
+  scoped_ptr<WebUIImpl> new_web_ui;
+  should_reuse_web_ui_ = ShouldReuseWebUI(current_navigation_entry, url);
+  if (!should_reuse_web_ui_)
+    new_web_ui = CreateWebUI(url, bindings);
+
+  int opener_route_id =
+      CreateOpenerRenderViewsIfNeeded(old_instance, new_instance);
+
+  int create_render_frame_flags = 0;
+  if (frame_tree_node_->IsMainFrame())
+    create_render_frame_flags |= CREATE_RF_FOR_MAIN_FRAME_NAVIGATION;
+  if (delegate_->IsHidden())
+    create_render_frame_flags |= CREATE_RF_HIDDEN;
+  scoped_ptr<RenderFrameHostImpl> new_render_frame_host =
+      CreateRenderFrame(new_instance, new_web_ui.get(), opener_route_id,
+                        create_render_frame_flags, nullptr);
+  if (!new_render_frame_host) {
+    return false;
+  }
+  speculative_render_frame_host_.reset(new_render_frame_host.release());
+  speculative_web_ui_.reset(new_web_ui.release());
+  return true;
+}
+
+// PlzNavigate
 RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
       const GURL& url,
       ui::PageTransition transition) {
   CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
-  // TODO(clamy): When we handle renderer initiated navigations, make sure not
-  // to use a different process for subframes if --site-per-process is not
-  // enabled.
-
   // Pick the right RenderFrameHost to commit the navigation.
-  // TODO(clamy): Replace the default values by the right ones.
-  RenderFrameHostImpl* render_frame_host = UpdateStateForNavigate(
-      url, NULL, transition, false, false, GlobalRequestID(),
-      NavigationEntryImpl::kInvalidBindings);
 
-  // If the renderer that needs to navigate is not live (it was just created or
-  // it crashed), initialize it.
-  if (!render_frame_host->render_view_host()->IsRenderViewLive()) {
-    // Recreate the opener chain.
-    int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager(
-        render_frame_host->GetSiteInstance());
-    if (!InitRenderView(render_frame_host->render_view_host(),
-                        opener_route_id,
-                        MSG_ROUTING_NONE,
-                        frame_tree_node_->IsMainFrame())) {
-      return NULL;
-    }
+  SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
+  // TODO(clamy): Replace the default values with the right ones for
+  // dest_instance, dest_is_restore, dest_is_view_source_mode.
+  scoped_refptr<SiteInstance> new_instance =
+      GetSiteInstanceForNavigation(url, NULL, transition, false, false);
+
+  // Will keep the current RFH if its SiteInstance matches the new one from
+  // the navigation or if this is a subframe navigation. If --site-per-process
+  // is enabled the RFH is never kept when sites don't match.
+  // TODO(carlosk): do not swap processes for renderer initiated navigations
+  // (see crbug.com/440266).
+  if (current_instance == new_instance.get() ||
+      (!frame_tree_node_->IsMainFrame() &&
+       !CommandLine::ForCurrentProcess()->HasSwitch(
+           switches::kSitePerProcess))) {
+    CleanUpNavigation();
+    return render_frame_host_.get();
+  }
+  // If the SiteInstance for the final URL doesn't match the one from the
+  // speculatively created RenderFrameHost, create a new one using the
+  // former.
+  if (!speculative_render_frame_host_ ||
+      speculative_render_frame_host_->GetSiteInstance() != new_instance.get()) {
+    CleanUpNavigation();
+    // TODO(clamy): Replace the binding value with the right one.
+    bool success = CreateSpeculativeRenderFrameHost(
+        url, current_instance, new_instance.get(),
+        NavigationEntryImpl::kInvalidBindings);
+    CHECK(success);
   }
-  return render_frame_host;
+  DCHECK(speculative_render_frame_host_);
+  return speculative_render_frame_host_.get();
+}
+
+// PlzNavigate
+void RenderFrameHostManager::CleanUpNavigation() {
+  scoped_ptr<RenderFrameHostImpl> rfh = UnsetSpeculativeRenderFrameHost();
+  if (rfh)
+    DiscardUnusedFrame(rfh.Pass());
+}
+
+// PlzNavigate
+scoped_ptr<RenderFrameHostImpl>
+RenderFrameHostManager::UnsetSpeculativeRenderFrameHost() {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  if (speculative_web_ui_)
+    speculative_web_ui_.reset();
+  should_reuse_web_ui_ = false;
+  if (speculative_render_frame_host_) {
+    speculative_render_frame_host_->GetProcess()->RemovePendingView();
+    return speculative_render_frame_host_.Pass();
+  }
+  return nullptr;
 }
 
 void RenderFrameHostManager::Observe(
@@ -1115,8 +1267,8 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
   if (view_routing_id_ptr)
     *view_routing_id_ptr = MSG_ROUTING_NONE;
 
-  // We are creating a pending or swapped out RFH here. We should never create
-  // it in the same SiteInstance as our current RFH.
+  // We are creating a pending, speculative or swapped out RFH here. We should
+  // never create it in the same SiteInstance as our current RFH.
   CHECK_NE(render_frame_host_->GetSiteInstance(), instance);
 
   // Check if we've already created an RFH for this SiteInstance.  If so, try
@@ -1293,29 +1445,39 @@ int RenderFrameHostManager::GetRoutingIdForSiteInstance(
 void RenderFrameHostManager::CommitPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CommitPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
+  bool browser_side_navigation = CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation);
   // First check whether we're going to want to focus the location bar after
   // this commit.  We do this now because the navigation hasn't formally
   // committed yet, so if we've already cleared |pending_web_ui_| the call chain
   // this triggers won't be able to figure out what's going on.
   bool will_focus_location_bar = delegate_->FocusLocationBarByDefault();
 
-  // Next commit the Web UI, if any. Either replace |web_ui_| with
-  // |pending_web_ui_|, or clear |web_ui_| if there is no pending WebUI, or
-  // leave |web_ui_| as is if reusing it.
-  DCHECK(!(pending_web_ui_.get() && pending_and_current_web_ui_.get()));
-  if (pending_web_ui_) {
-    web_ui_.reset(pending_web_ui_.release());
-  } else if (!pending_and_current_web_ui_.get()) {
-    web_ui_.reset();
+  if (!browser_side_navigation) {
+    DCHECK(!speculative_web_ui_);
+    // Next commit the Web UI, if any. Either replace |web_ui_| with
+    // |pending_web_ui_|, or clear |web_ui_| if there is no pending WebUI, or
+    // leave |web_ui_| as is if reusing it.
+    DCHECK(!(pending_web_ui_.get() && pending_and_current_web_ui_.get()));
+    if (pending_web_ui_) {
+      web_ui_.reset(pending_web_ui_.release());
+    } else if (!pending_and_current_web_ui_.get()) {
+      web_ui_.reset();
+    } else {
+      DCHECK_EQ(pending_and_current_web_ui_.get(), web_ui_.get());
+      pending_and_current_web_ui_.reset();
+    }
   } else {
-    DCHECK_EQ(pending_and_current_web_ui_.get(), web_ui_.get());
-    pending_and_current_web_ui_.reset();
+    // PlzNavigate
+    if (!should_reuse_web_ui_)
+      web_ui_.reset(speculative_web_ui_.release());
+    DCHECK(!speculative_web_ui_);
   }
 
   // It's possible for the pending_render_frame_host_ to be NULL when we aren't
   // crossing process boundaries. If so, we just needed to handle the Web UI
   // committing above and we're done.
-  if (!pending_render_frame_host_) {
+  if (!pending_render_frame_host_ && !speculative_render_frame_host_) {
     if (will_focus_location_bar)
       delegate_->SetFocusToLocationBar(false);
     return;
@@ -1329,10 +1491,20 @@ void RenderFrameHostManager::CommitPending() {
 
   bool is_main_frame = frame_tree_node_->IsMainFrame();
 
-  // Swap in the pending frame and make it active. Also ensure the FrameTree
-  // stays in sync.
-  scoped_ptr<RenderFrameHostImpl> old_render_frame_host =
-      SetRenderFrameHost(pending_render_frame_host_.Pass());
+  scoped_ptr<RenderFrameHostImpl> old_render_frame_host;
+  if (!browser_side_navigation) {
+    DCHECK(!speculative_render_frame_host_);
+    // Swap in the pending frame and make it active. Also ensure the FrameTree
+    // stays in sync.
+    old_render_frame_host =
+        SetRenderFrameHost(pending_render_frame_host_.Pass());
+  } else {
+    // PlzNavigate
+    DCHECK(speculative_render_frame_host_);
+    old_render_frame_host =
+        SetRenderFrameHost(speculative_render_frame_host_.Pass());
+  }
+
   if (is_main_frame)
     render_frame_host_->render_view_host()->AttachToFrameTree();