PlzNavigate: Speculatively spawns a renderer process for navigations.

content/browser/frame_host/render_frame_host_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/render_frame_host_manager.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 42 matching lines @@
     RenderFrameHostDelegate* render_frame_delegate,
     RenderViewHostDelegate* render_view_delegate,
     RenderWidgetHostDelegate* render_widget_delegate,
     Delegate* delegate)
     : frame_tree_node_(frame_tree_node),
       delegate_(delegate),
       cross_navigation_pending_(false),
       render_frame_delegate_(render_frame_delegate),
       render_view_delegate_(render_view_delegate),
       render_widget_delegate_(render_widget_delegate),
-      interstitial_page_(NULL),
+      interstitial_page_(nullptr),
+      should_reuse_web_ui_(false),
       weak_factory_(this) {
   DCHECK(frame_tree_node_);
 }
 
 RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_)
     UnsetPendingRenderFrameHost();
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(

[Charlie Reis, 2015/01/12 23:30:52]

We should probably mimic the case above and check for
speculative_render_frame_host_ instead of the switch.  (No need for braces.)

[carlosk, 2015/01/13 15:54:13]

Done. 

I was in doubt about this one because we were almost always using the switch
guard.

+          switches::kEnableBrowserSideNavigation)) {
+    UnsetSpeculativeRenderFrameHost();
+  }
+
   if (render_frame_host_ &&
       render_frame_host_->GetSiteInstance()->active_frame_count() <= 1U) {
     ShutdownRenderFrameProxyHostsInSiteInstance(
         render_frame_host_->GetSiteInstance()->GetId());
   }
 
   // We should always have a current RenderFrameHost except in some tests.
   SetRenderFrameHost(scoped_ptr<RenderFrameHostImpl>());
 
   // Delete any swapped out RenderFrameHosts.
@@ skipping 203 matching lines @@
     }
   }
   return false;
 }
 
 void RenderFrameHostManager::OnBeforeUnloadACK(
     bool for_cross_site_transition,
     bool proceed,
     const base::TimeTicks& proceed_time) {
   if (for_cross_site_transition) {
+    DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kEnableBrowserSideNavigation));
     // Ignore if we're not in a cross-site navigation.
     if (!cross_navigation_pending_)
       return;
 
     if (proceed) {
       // Ok to unload the current page, so proceed with the cross-site
       // navigation.  Note that if navigations are not currently suspended, it
       // might be because the renderer was deemed unresponsive and this call was
       // already made by ShouldCloseTabOnUnresponsiveRenderer.  In that case, it
       // is ok to do nothing here.
@@ skipping 106 matching lines @@
   response_started_id_.reset();
 }
 
 void RenderFrameHostManager::ClearNavigationTransitionData() {
   render_frame_host_->ClearPendingTransitionRequestData();
 }
 
 void RenderFrameHostManager::DidNavigateFrame(
     RenderFrameHostImpl* render_frame_host,
     bool was_caused_by_user_gesture) {
+  DCHECK(render_frame_host);

[Charlie Reis, 2015/01/12 23:30:52]

This seems unnecessary to me, since the callers are all within the navigation
logic and it's fairly obvious they won't pass in null.  (We don't have DCHECKs
in those methods either.)

[carlosk, 2015/01/13 15:54:13]

Done.

+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    if (render_frame_host == speculative_render_frame_host_.get()) {
+      CommitPending();
+    } else if (render_frame_host == render_frame_host_.get()) {
+      // TODO(carlosk): this code doesn't properly handles in-page nor

[Charlie Reis, 2015/01/12 23:30:52]

handle
nor -> or

[carlosk, 2015/01/13 15:54:13]

Done.

+      // interweaved navigation requests navigation.

[Charlie Reis, 2015/01/12 23:30:52]

I don't know what this means.

[carlosk, 2015/01/13 15:54:13]

Fixed: /interweaved/interwoven

This case might happen if one browser-initiated navigation is at the point of
waiting for the renderer to commit when another browser-initiated navigation is
started. Then we received here the reply from the renderer regarding the 1st
navigation and the |render_frame_host| might not match the new speculative that
belongs to the 2nd one.

Both this case and in-page navigation might fail the DCHECK here.

+      DCHECK(!speculative_render_frame_host_);
+    } else {
+      // No one else should be sending us a DidNavigate in this state.
+      DCHECK(false);
+    }
+    DCHECK(!speculative_render_frame_host_);
+    return;
+  }
+
   if (!cross_navigation_pending_) {

[Charlie Reis, 2015/01/12 23:30:52]

Why isn't this block above the one you added?  Is cross_navigation_pending_
never set to true in PlzNavigate mode?  If not, we should document that in the
.h file, since it's checked in a ton of places.  We should (later) make sure
that doesn't cause a problem, if it's true.

[carlosk, 2015/01/13 15:54:12]

It is not used by PlzNavigate so I documented that in the .h file.

The only two methods that (I guess) are still in use with PlzNavigate where
cross_navigation_pending_ is checked are RFHM::Stop and
RFHM::ShouldCloseTabOnUnresponsiveRenderer. RFHM::OnBeforeUnloadACK also uses it
but in a section not active with PlzNavigate (see DCHECK).

[Charlie Reis, 2015/01/13 18:48:21]

Ok.  Those cases will matter, but it sounds like you're tracking them, so that's
fine for now.

[carlosk, 2015/01/14 10:09:47]

Acknowledged.

     DCHECK(!pending_render_frame_host_);
 
     // We should only hear this from our current renderer.
     DCHECK_EQ(render_frame_host_, render_frame_host);
 
     // Even when there is no pending RVH, there may be a pending Web UI.
     if (pending_web_ui())
       CommitPending();
     return;
   }
 
   if (render_frame_host == pending_render_frame_host_) {
     // The pending cross-site navigation completed, so show the renderer.
     CommitPending();
     cross_navigation_pending_ = false;
   } else if (render_frame_host == render_frame_host_) {
     if (was_caused_by_user_gesture) {
       // A navigation in the original page has taken place.  Cancel the pending
       // one. Only do it for user gesture originated navigations to prevent
       // page doing any shenanigans to prevent user from navigating.
-      // See https://code.google.com/p/chromium/issues/detail?id=75195
+      // See https://crbug.com/75195.

[Charlie Reis, 2015/01/12 23:30:53]

I have no problem with this change in general, but it's unrelated to this CL. 
Given how many revisions and rebases we've been through (and the chance that
this might be reverted), we really should not be including any unnecessary churn
in this CL.

[carlosk, 2015/01/13 15:54:12]

Acknowledged and reverted. (FYI I was asked for this change)

       CancelPending();
       cross_navigation_pending_ = false;
     }
   } else {
     // No one else should be sending us DidNavigate in this state.
     DCHECK(false);
   }
 }
 
 void RenderFrameHostManager::DidDisownOpener(
@@ skipping 118 matching lines @@
   // If the SiteInstance for the pending RFH is being used by others don't
   // delete the RFH. Just swap it out and it can be reused at a later point.
   SiteInstanceImpl* site_instance = render_frame_host->GetSiteInstance();
   if (site_instance->HasSite() && site_instance->active_frame_count() > 1) {
     // Any currently suspended navigations are no longer needed.
     render_frame_host->CancelSuspendedNavigations();
 
     RenderFrameProxyHost* proxy =
         new RenderFrameProxyHost(site_instance, frame_tree_node_);
     proxy_hosts_[site_instance->GetId()] = proxy;
-    render_frame_host->SwapOut(proxy, false);
+
+    // As the RenderFrameHost might already be swapped out, check if that is

[Charlie Reis, 2015/01/12 23:30:52]

Simplify: "Check if the RenderFrameHost is already swapped out, to avoid
swapping it out again."

[carlosk, 2015/01/13 15:54:13]

Done.

+    // the case to avoid swapping it out again.
+    if (!render_frame_host->is_swapped_out())
+      render_frame_host->SwapOut(proxy, false);
+
     if (frame_tree_node_->IsMainFrame())
       proxy->TakeFrameHostOwnership(render_frame_host.Pass());
   } else {
     // We won't be coming back, so delete this one.
     render_frame_host.reset();
   }
 }
 
 void RenderFrameHostManager::MoveToPendingDeleteHosts(
     scoped_ptr<RenderFrameHostImpl> render_frame_host) {
@@ skipping 24 matching lines @@
     }
   }
   return false;
 }
 
 void RenderFrameHostManager::ResetProxyHosts() {
   STLDeleteValues(&proxy_hosts_);
 }
 
 // PlzNavigate
+void RenderFrameHostManager::BeginNavigation(
+    const CommonNavigationParams& common_params) {
+  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  // Cleans up any state in case there's an ongoing navigation.

[Charlie Reis, 2015/01/12 23:30:53]

nit: Clean

[carlosk, 2015/01/13 15:54:13]

Done.

+  // TODO(carlosk): remove this cleanup here once we properly cancel ongoing
+  // navigations.
+  CleanUpNavigation();
+
+  RenderFrameHostImpl* future_rfh =

[Charlie Reis, 2015/01/12 23:30:52]

future_rfh -> dest_rfh

Also, it's worth mentioning in a comment (or a DCHECK?) that this is either the
current or speculative RFH, and thus it is already owned.

[carlosk, 2015/01/13 15:54:13]

Done the name change.

And for the expectation you described I updated a DCHECK in
GetFrameHostForNavigation and improved its doc comment in the .h file  to
reflect it.

+      GetFrameHostForNavigation(common_params.url, common_params.transition);
+  DCHECK(future_rfh);
+}
+
+// PlzNavigate
 RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
-      const GURL& url,
-      ui::PageTransition transition) {
+    const GURL& url,
+    ui::PageTransition transition) {
   CHECK(base::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.
+  RenderFrameHostImpl* rfh_for_navigation = nullptr;

[Charlie Reis, 2015/01/12 23:30:52]

new_rfh would better match new_instance.  Please also put their declarations
together.

[carlosk, 2015/01/13 15:54:13]

I went another route:
current_instance -> current_site_instance
new_instance -> navigation_site_instance
rfh_for_navigation -> navigation_rfh

I disliked "new" as they might not be actually new and the *_instance suffix was
misleading because it invokes the OO concept.

WDYT?

[Charlie Reis, 2015/01/13 18:48:21]

*_site_instance instead of *_instance is fine.

I probably would choose "dest_" instead of "navigation_" for brevity and
consistency with existing names.  I agree with "new" being potentially
confusing.

[carlosk, 2015/01/14 10:09:46]

Done: navigation_site_instance -> dest_site_instance

I kept the navigation_rfh as it matches the method name.

 
-  // Pick the right RenderFrameHost to commit the navigation.
-  // TODO(clamy): Replace the default values by the right ones.
-  RenderFrameHostImpl* render_frame_host = UpdateStateForNavigate(
-      url, nullptr, nullptr, transition, false, false, GlobalRequestID(),
-      NavigationEntryImpl::kInvalidBindings);
+  SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
 
-  // 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()) {
+  // TODO(carlosk): Replace the default values with the right ones for
+  // source_instance, dest_instance, dest_is_restore, dest_is_view_source_mode.
+  scoped_refptr<SiteInstance> new_instance = GetSiteInstanceForNavigation(
+      url, nullptr, nullptr, transition, false, false);
+
+  // TODO(carlosk): do not swap processes for renderer initiated navigations
+  // (see crbug.com/440266).
+  if (current_instance == new_instance.get() ||
+      (!frame_tree_node_->IsMainFrame() &&
+       !base::CommandLine::ForCurrentProcess()->HasSwitch(
+           switches::kSitePerProcess))) {
+    // Will reuse the current RFH if its SiteInstance matches the new one from

[Charlie Reis, 2015/01/12 23:30:53]

Drop "Will"

[carlosk, 2015/01/13 15:54:12]

Done. Also updated the rest a bit.

+    // 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.

[Charlie Reis, 2015/01/12 23:30:52]

This isn't quite true, since we don't plan to isolate every site with
--site-per-process.  Let's rephrase the last sentence to "We only swap RFHs for
subframes when --site-per-process is enabled."

[carlosk, 2015/01/13 15:54:12]

Done.

+    CleanUpNavigation();
+    rfh_for_navigation = render_frame_host_.get();
+  } else {
+    // If the current render_frame_host_ isn't live, we should create it so
+    // that we don't show a sad tab while the navigation is ongoing.
+    // (Bug 1145340)
+    if (!render_frame_host_->IsRenderFrameLive()) {
+      // Note: we don't call InitRenderView here because we are navigating away
+      // soon anyway, and we don't have the NavigationEntry for this host.
+      delegate_->CreateRenderViewForRenderManager(
+          render_frame_host_->render_view_host(), MSG_ROUTING_NONE,
+          MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame());
+    }
+
+    // If the SiteInstance for the final URL doesn't match the one from the
+    // speculatively created RenderFrameHost, create a new RenderFrameHost using
+    // this new SiteInstance.
+    if (!speculative_render_frame_host_ ||
+        speculative_render_frame_host_->GetSiteInstance() !=
+            new_instance.get()) {
+      CleanUpNavigation();
+      // TODO(carlosk): Replace the binding value with the right one.
+      bool success = CreateSpeculativeRenderFrameHost(
+          url, current_instance, new_instance.get(),
+          NavigationEntryImpl::kInvalidBindings);
+      DCHECK(success);
+    }
+    DCHECK(speculative_render_frame_host_);
+    rfh_for_navigation = speculative_render_frame_host_.get();
+  }
+  DCHECK(rfh_for_navigation);
+
+  // If the renderer that needs to navigate is not live (it was just created
+  // or it crashed), initialize it.
+  if (!rfh_for_navigation->render_view_host()->IsRenderViewLive()) {

[Charlie Reis, 2015/01/12 23:30:52]

Why not IsRenderFrameLive() on the RFH, as above?

[carlosk, 2015/01/13 15:54:12]

Replaced. I think I copied these two snippets from somewhere else so they came
as they were.

[Charlie Reis, 2015/01/13 18:48:21]

Oh, I see.  I guess we left that case as IsRenderViewLive because the calls made
in this block are about RenderViews.

I suppose either way is fine by me, since I think they should be equivalent at
this point.

[carlosk, 2015/01/14 10:09:46]

Acknowledged.

     // 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())) {
+        rfh_for_navigation->GetSiteInstance());
+    if (!InitRenderView(rfh_for_navigation->render_view_host(), opener_route_id,
+                        MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame())) {
       return nullptr;
     }
   }
-  return render_frame_host;
+
+  return rfh_for_navigation;
+}
+
+// PlzNavigate
+void RenderFrameHostManager::CleanUpNavigation() {
+  scoped_ptr<RenderFrameHostImpl> rfh = UnsetSpeculativeRenderFrameHost();

[Charlie Reis, 2015/01/12 23:30:52]

As mentioned below, this should be:
if (speculative_render_frame_host_)
  DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());

[carlosk, 2015/01/13 15:54:13]

The reason I did it this way was to cover the case of the web_ui being set but
not the speculative RFH. But it doesn't seem like that's possible so I made the
change as you suggested.

But this made me be a bit more cautious with setting |should_reuse_web_ui_| so I
made a little change to CreateSpeculativeRenderFrameHost.

[Charlie Reis, 2015/01/13 18:48:21]

Acknowledged.

+  if (rfh)
+    DiscardUnusedFrame(rfh.Pass());
+}
+
+// PlzNavigate
+scoped_ptr<RenderFrameHostImpl>
+RenderFrameHostManager::UnsetSpeculativeRenderFrameHost() {
+  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  if (speculative_web_ui_)
+    speculative_web_ui_.reset();
+  should_reuse_web_ui_ = false;
+  if (speculative_render_frame_host_) {

[Charlie Reis, 2015/01/12 23:30:52]

Seems like we shouldn't call this unless there is a speculative RFH (similar
UnsetPendingRenderFrameHost).

[carlosk, 2015/01/13 15:54:12]

Now this one sounds a bit too risky for the potential null dereference crash
here (thinking defensive programming). I prefer not to cause a crash like this
in a cleanup function, WDYT?

[Charlie Reis, 2015/01/13 18:48:21]

I disagree.  If we're calling this method with no speculative RFH, then
something is wrong and it's better to crash early and get a report.  Please make
it consistent with UnsetPendingRenderFrameHost.

[carlosk, 2015/01/14 10:09:47]

Done.

+    speculative_render_frame_host_->GetProcess()->RemovePendingView();
+    return speculative_render_frame_host_.Pass();
+  }
+  return nullptr;
 }
 
 void RenderFrameHostManager::OnDidStartLoading() {
   for (const auto& pair : proxy_hosts_) {
     pair.second->Send(
         new FrameMsg_DidStartLoading(pair.second->GetRoutingID()));
   }
 }
 
 void RenderFrameHostManager::OnDidStopLoading() {
@@ skipping 385 matching lines @@
 
   if (delegate_->IsHidden())
     create_render_frame_flags |= CREATE_RF_HIDDEN;
 
   int opener_route_id =
       CreateOpenerRenderViewsIfNeeded(old_instance, new_instance);
 
   if (pending_render_frame_host_)
     CancelPending();
 
-  // Create a non-swapped-out RFH with the given opener.
+  // Create a non swapped out RFH with the given opener.

[Charlie Reis, 2015/01/12 23:30:52]

No extra churn, please.

[carlosk, 2015/01/13 15:54:13]

Done.

   pending_render_frame_host_ =
       CreateRenderFrame(new_instance, pending_web_ui(), opener_route_id,
                         create_render_frame_flags, nullptr);
 }
 
 int RenderFrameHostManager::CreateOpenerRenderViewsIfNeeded(
     SiteInstance* old_instance,
     SiteInstance* new_instance) {
   int opener_route_id = MSG_ROUTING_NONE;
   if (new_instance->IsRelatedSiteInstance(old_instance)) {
@@ skipping 16 matching lines @@
     int frame_routing_id,
     int flags) {
   if (frame_routing_id == MSG_ROUTING_NONE)
     frame_routing_id = site_instance->GetProcess()->GetNextRoutingID();
 
   bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
   bool hidden = !!(flags & CREATE_RF_HIDDEN);
 
   // Create a RVH for main frames, or find the existing one for subframes.
   FrameTree* frame_tree = frame_tree_node_->frame_tree();
-  RenderViewHostImpl* render_view_host = NULL;
+  RenderViewHostImpl* render_view_host = nullptr;
   if (frame_tree_node_->IsMainFrame()) {
     render_view_host = frame_tree->CreateRenderViewHost(
         site_instance, view_routing_id, frame_routing_id, swapped_out, hidden);
   } else {
     render_view_host = frame_tree->GetRenderViewHost(site_instance);
 
     CHECK(render_view_host);
   }
 
   // TODO(creis): Pass hidden to RFH.
   scoped_ptr<RenderFrameHostImpl> render_frame_host =
       make_scoped_ptr(RenderFrameHostFactory::Create(
                           render_view_host, render_frame_delegate_, frame_tree,
                           frame_tree_node_, frame_routing_id, flags).release());
   return render_frame_host.Pass();
 }
 
+// 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_ = new_render_frame_host.Pass();
+  speculative_web_ui_ = new_web_ui.Pass();
+  return true;
+}
+
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     SiteInstance* instance,
     WebUIImpl* web_ui,
     int opener_route_id,
     int flags,
     int* view_routing_id_ptr) {
   bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
   CHECK(instance);
   // Swapped out views should always be hidden.
   DCHECK(!swapped_out || (flags & CREATE_RF_HIDDEN));
 
   // TODO(nasko): Remove the following CHECK once cross-site navigation no
   // longer relies on swapped out RFH for the top-level frame.
   if (!frame_tree_node_->IsMainFrame())
     CHECK(!swapped_out);
 
   scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
   bool success = true;
   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
   // to re-use the existing one, which has already been initialized.  We'll
   // remove it from the list of proxy hosts below if it will be active.
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
   if (proxy && proxy->render_frame_host()) {
     if (view_routing_id_ptr)
       *view_routing_id_ptr = proxy->GetRenderViewHost()->GetRoutingID();
     // Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
@@ skipping 92 matching lines @@
 
 bool RenderFrameHostManager::InitRenderView(
     RenderViewHostImpl* render_view_host,
     int opener_route_id,
     int proxy_routing_id,
     bool for_main_frame_navigation) {
   // We may have initialized this RenderViewHost for another RenderFrameHost.
   if (render_view_host->IsRenderViewLive())
     return true;
 
-  // If the pending navigation is to a WebUI and the RenderView is not in a
+  // If the ongoing navigation is to a WebUI and the RenderView is not in a
   // guest process, tell the RenderViewHost about any bindings it will need
   // enabled.
-  if (pending_web_ui() && !render_view_host->GetProcess()->IsIsolatedGuest()) {
-    render_view_host->AllowBindings(pending_web_ui()->GetBindings());
+  WebUIImpl* future_web_ui;

[Charlie Reis, 2015/01/12 23:30:52]

dest_web_ui = nullptr;

[carlosk, 2015/01/13 15:54:13]

Done.

+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    future_web_ui =
+        should_reuse_web_ui_ ? web_ui_.get() : speculative_web_ui_.get();
+  } else {
+    future_web_ui = pending_web_ui();
+  }
+  if (future_web_ui && !render_view_host->GetProcess()->IsIsolatedGuest()) {
+    render_view_host->AllowBindings(future_web_ui->GetBindings());
   } else {
     // Ensure that we don't create an unprivileged RenderView in a WebUI-enabled
     // process unless it's swapped out.
     if (render_view_host->is_active()) {
       CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
                 render_view_host->GetProcess()->GetID()));
     }
   }
 
   return delegate_->CreateRenderViewForRenderManager(render_view_host,
@@ skipping 41 matching lines @@
       proxy_hosts_.find(site_instance->GetId());
   if (iter != proxy_hosts_.end())
     return iter->second->GetRoutingID();
 
   return MSG_ROUTING_NONE;
 }
 
 void RenderFrameHostManager::CommitPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CommitPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
+  bool browser_side_navigation =
+      base::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_) {
+  // It's possible for the pending_render_frame_host_ to be nullptr 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_ && !speculative_render_frame_host_) {
     if (will_focus_location_bar)
       delegate_->SetFocusToLocationBar(false);
     return;
   }
 
   // Remember if the page was focused so we can focus the new renderer in
   // that case.
   bool focus_render_view = !will_focus_location_bar &&
       render_frame_host_->render_view_host()->GetView() &&
       render_frame_host_->render_view_host()->GetView()->HasFocus();
 
   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());
+  // Swap in the pending or speculative frame and make it active. Also ensure
+  // the FrameTree stays in sync.
+  scoped_ptr<RenderFrameHostImpl> old_render_frame_host;
+  if (!browser_side_navigation) {
+    DCHECK(!speculative_render_frame_host_);
+    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();
 
   // The process will no longer try to exit, so we can decrement the count.
   render_frame_host_->GetProcess()->RemovePendingView();
 
   // Show the new view (or a sad tab) if necessary.
   bool new_rfh_has_view = !!render_frame_host_->render_view_host()->GetView();
   if (!delegate_->IsHidden() && new_rfh_has_view) {
     // In most cases, we need to show the new view.
@@ skipping 125 matching lines @@
     TRACE_EVENT_INSTANT2(
         "navigation",
         "RenderFrameHostManager::UpdateStateForNavigate:New SiteInstance",
         TRACE_EVENT_SCOPE_THREAD,
         "current_instance id", current_instance->GetId(),
         "new_instance id", new_instance->GetId());
 
     // New SiteInstance: create a pending RFH to navigate.
     DCHECK(!cross_navigation_pending_);
 
-    // This will possibly create (set to NULL) a Web UI object for the pending
-    // page. We'll use this later to give the page special access. This must
-    // happen before the new renderer is created below so it will get bindings.
-    // It must also happen after the above conditional call to CancelPending(),
-    // otherwise CancelPending may clear the pending_web_ui_ and the page will
-    // not have its bindings set appropriately.
+    // This will possibly create (set to nullptr) a Web UI object for the
+    // pending page. We'll use this later to give the page special access. This
+    // must happen before the new renderer is created below so it will get
+    // bindings. It must also happen after the above conditional call to
+    // CancelPending(), otherwise CancelPending may clear the pending_web_ui_
+    // and the page will not have its bindings set appropriately.
     SetPendingWebUI(dest_url, bindings);
     CreatePendingRenderFrameHost(current_instance, new_instance.get(),
                                  frame_tree_node_->IsMainFrame());
     if (!pending_render_frame_host_.get()) {
-      return NULL;
+      return nullptr;
     }
 
     // Check if our current RFH is live before we set up a transition.
     if (!render_frame_host_->IsRenderFrameLive()) {
       if (!cross_navigation_pending_) {
         // The current RFH is not live.  There's no reason to sit around with a
         // sad tab or a newly created RFH while we wait for the pending RFH to
         // navigate.  Just switch to the pending RFH now and go back to non
         // cross-navigating (Note that we don't care about on{before}unload
         // handlers if the current RFH isn't live.)
         CommitPending();
         return render_frame_host_.get();
       } else {
         NOTREACHED();
         return render_frame_host_.get();
       }
     }
     // Otherwise, it's safe to treat this as a pending cross-site transition.
 
     // We now have a pending RFH.
     DCHECK(!cross_navigation_pending_);
     cross_navigation_pending_ = true;
 
-    // PlzNavigate: There is no notion of transfer navigations, and the old
-    // renderer before unload handler has already run at that point, so return
-    // here.
-    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kEnableBrowserSideNavigation)) {
-      return pending_render_frame_host_.get();
-    }
-
     // We need to wait until the beforeunload handler has run, unless we are
     // transferring an existing request (in which case it has already run).
     // Suspend the new render view (i.e., don't let it send the cross-site
     // Navigate message) until we hear back from the old renderer's
     // beforeunload handler.  If the handler returns false, we'll have to
     // cancel the request.
     //
     DCHECK(!pending_render_frame_host_->are_navigations_suspended());
     bool is_transfer = transferred_request_id != GlobalRequestID();
     if (is_transfer) {
@@ skipping 154 matching lines @@
 void RenderFrameHostManager::DeleteRenderFrameProxyHost(
     SiteInstance* instance) {
   RenderFrameProxyHostMap::iterator iter = proxy_hosts_.find(instance->GetId());
   if (iter != proxy_hosts_.end()) {
     delete iter->second;
     proxy_hosts_.erase(iter);
   }
 }
 
 }  // namespace content