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 43 matching lines @@
     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),
+      should_reuse_web_ui_(false),
       weak_factory_(this) {
   DCHECK(frame_tree_node_);
 }
 
 RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_)
     CancelPending();
 
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    CleanUpNavigation();

[ncarter (slow), 2014/12/12 22:37:54]

A CL I'm working on -- https://codereview.chromium.org/799593004/ -- will fix a
bug in ~RenderFrameHostManager that's due to the save-for-reuse work of
DiscardUnusedFrame being problematic during destruction.

Since CleanUpNavigation() also calls DiscardUnusedFrame, I think this will
introduce a new instance of that bug. See if you can use the test I added in the
CL as a starting point for tickling the CleanUpNavigation() instance?

[carlosk, 2014/12/16 01:53:48]

Thanks for pointing this out! I refactored the code to follow pretty much
exactly your fix for the pending case.

+  }
+
   // We should always have a current RenderFrameHost except in some tests.
   SetRenderFrameHost(scoped_ptr<RenderFrameHostImpl>());
 
   // Delete any swapped out RenderFrameHosts.
   STLDeleteValues(&proxy_hosts_);
 }
 
 void RenderFrameHostManager::Init(BrowserContext* browser_context,
                                   SiteInstance* site_instance,
                                   int view_routing_id,
@@ skipping 199 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(!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 111 matching lines @@
 
   response_started_id_.reset();
 }
 
 void RenderFrameHostManager::ClearNavigationTransitionData() {
   render_frame_host_->ClearPendingTransitionRequestData();
 }
 
 void RenderFrameHostManager::DidNavigateFrame(
     RenderFrameHostImpl* render_frame_host) {
+  DCHECK(render_frame_host);
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    return;
+  }
+
   if (!cross_navigation_pending_) {
     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_) {

[clamy, 2014/12/09 15:09:52]

We probably want to something a bit similar, only with the speculative RFH
instead of the pending one. I am not sure we can quite reuse that one, due to
slight differences in the case of same-site navigations (see below).

[carlosk, 2014/12/16 01:53:48]

This makes me think that even after committing a specific RFH we could get a
different one here. Isn't this awfully weird? And I'm guessing we want to change
this with PlzNavigate?

Also I don't know if this is lacking support by our test infrastructure or if
it's PlzNavigate that is still incomplete, but this method always receives the
currently active RFH instead of the one that was committed before. So I tried
adding this DCHECK:

DCHECK((render_frame_host == speculative_render_frame_host_.get()) ||
       (render_frame_host == render_frame_host_.get() &&
            !speculative_render_frame_host_));

but it would just fail all the time because of that.

I'm adding TODOs and we'll have to figure this out later. Maybe the solution is
simply to remove the RenderFrameHostImpl parameter?

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

No we can only get the pending RFH or current RFH with the way the code
currently works. And with the way PlzNavigate should work, we should also only
get the speculative or the current one. I don't really understand what you mean
by RFH we commit. We commit the RFH that sends us this IPC.

As to reason the test is failing, it is most likely because the tests make use
of TestWebContents::NavigateAndCommit. This function itself calls
TestWebContents::CommitPendingNavigation. The way the function works is by doing
roughly this:
TestRenderFrameHost* old_rfh = GetMainFrame();
TestRenderFrameHost* rfh = GetPendingMainFrame();
if (!rfh)
  rfh = old_rfh;

rfh->SendDidNavigate

It needs to be updated to take into account the speculative RFH instead of the
pending one when PlzNavigate is enabled (otherwise since the pending rfh is
always null, we will only call DidNavigate with the current rfh).

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

Now that I read all of your other comments I realized that this method can be
called by the renderer without being triggered from the browser by an ongoing
navigation.
I see. I added the speculative RFH support there but that was not enough as we
were calling SendNavigate always on the main RFH. So I also switched to using
TestWebContents::CommitPendingNavigation so that the correct RFH commits but
that made me learn that as we were bypassing the controller the NavigationEntry
was not being properly set.

So I added a new test helper method, LoadURLAndBeginNavigationOnMainFrame, that
most tests now use (when they  don't need special navigation parameters) that
calls the controller's LoadUrl and also SendBeginNavigationWithURL... And
finally it worked. Whew!

As a side thing I added a new method to TestWebContents to get the main frame's
speculative RFH in the model of the existing one for pending. I didn't remove
the one we already had in NavigatorTestWithBrowserSideNavigation because it's
still needed for special cases (subframes).

     // The pending cross-site navigation completed, so show the renderer.
     CommitPending();
     cross_navigation_pending_ = false;
   } else if (render_frame_host == render_frame_host_) {

[clamy, 2014/12/09 15:09:52]

Note that in the PlzNavigate case, we only want to cancel the current navigation
(and speculative RFH) if we receive a commit message from an in-page
user-initiated navigation. In-page non user-initiated navigations should not
cancel any pending navigations, and in all other cases the pending navigation
should be the one that commits, and there should be no speculative RFH.

Also, CancelPending destroys the pending RFH, which indeed cancels an on-going
cross-site navigation. In PlzNavigate it does not work that way, so we need to
make sure that navigations are properly cancelled following the commit of
in-page user-initiated navigations. I think this can be left as a TODO for a
future patch though.

[carlosk, 2014/12/16 01:53:48]

Let me try to understand what you are saying. You are referring to the case
where an ongoing navigation was cancelled by a new request but this method is
called anyway for the cancelled one as the renderer was already processing it.
Is this so?

I think we'll have to figure out how to deal with that when we handle
cancellations properly and I'd really like to do this using the navigation
request ID. I added a TODO for this too.

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

I've written what I meant in code version in the comment above, so it should be
easier to understand. I left the cancelation part as a TODO. We cannot use
navigation request ID as those were removed. Instead, we just need to destroy
the navigation request for the FrameTreeNode that is stored in NavigatorImpl.

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

Acknowledged.

Note: Your reference to the "code above" confused the hell out of me as the
comment you are actually referring to is in the next PatchSet. :)

     // A navigation in the original page has taken place.  Cancel the pending
     // one.
     CancelPending();
     cross_navigation_pending_ = false;
   } else {
     // No one else should be sending us DidNavigate in this state.
     DCHECK(false);
   }
 }
 
@@ 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);
+
+    if (!render_frame_host->is_swapped_out())
+      render_frame_host->SwapOut(proxy);
+
     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 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.
+  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());
+      if (!InitRenderView(render_frame_host_->render_view_host(),
+                          opener_route_id, MSG_ROUTING_NONE,
+                          frame_tree_node_->IsMainFrame())) {
+        return;

[carlosk, 2014/12/09 07:55:43]

These are the returns I'd like to eliminate but I'd have to use if-blocks to
hold DCHECKs.

[clamy, 2014/12/09 15:09:52]

While it is fine in general to have an if-block with only a DCHECK inside, I
don't think it would be better in that case. IMO the early return makes it
clearer that something went wrong if InitRenderView did not succeed, compared to
writing:
if (InitRenderView(...))
  DCHECK(current_instance->GetProcess()->HasConnection())
return;

[Charlie Reis, 2014/12/10 22:37:45]

+1 to both.

(DCHECKs inside a conditional mostly make sense if the conditional can be
optimized out.)

[carlosk, 2014/12/16 01:53:48]

Finally I didn't need the if's. I simply DCHECK the |success|es.

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

That is not equivalent to the previous condition.

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

You are right, it is not equivalent for the Debug case as it will crash on
renderer creation failure.

Now for the Release case it is still equivalent.

As this version has stronger guarantees and content tests are still passing I
thought it was OK to keep it that way. WDYT?

+      }
+    }
+    DCHECK(current_instance->GetProcess()->HasConnection());
+    return;
+  }
+  // Speculatively create a new RFH.

[clamy, 2014/12/09 15:09:52]

Please add a line before that comment and remove the one below.

[carlosk, 2014/12/16 01:53:48]

Done.

+
+  // TODO(carlosk): enable bindings check below.
+  bool success = CreateSpeculativeRenderFrameHost(
+      common_params.url, current_instance, new_instance.get(),
+      NavigationEntryImpl::kInvalidBindings);
+  if (!success)
+    return;
+  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(

[clamy, 2014/12/09 15:09:52]

Please modify this function so that it does not commit the pending RFH. Instead
it should either return the current renderer, or the speculative RFH. If it
turns out we need a speculative RFH and we don't have one, or the one we have is
wrong, a new speculative RFh should be created and returned.

[carlosk, 2014/12/16 01:53:48]

Done.

       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.

[carlosk, 2014/12/09 07:55:43]

I removed this TODO in favor of the new ones I'm adding related to the newly
created bug.

But from the CR comments I had understood this policy applied for any kind of
renderer-initiated navigations, and not only for subframe. Is that so?

+  // Pick the right RenderFrameHost to commit the navigation.
 
-  // 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);
+  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();
+  } else {
+    // 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);
+    }
+    DCHECK(speculative_render_frame_host_);
+    CommitPending();

[nasko, 2014/12/10 23:11:34]

Why is this method calling CommitPending?

[carlosk, 2014/12/16 01:53:48]

Because it was here that we would switch the active RFHs. It was moved back to
the later step at DidNavigateFrame as per other discussions.

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

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

As mentionned above this _must_ absolutely stay there. We can't have an
initialized renderer try to commit the page. The renderer selected here is going
to try to request the navigation url from the browser and get the body of the
response (and it absolutely needs to be live to do that).

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

Acknowledged and moved it back here from DidNavigateFrame.

   // it crashed), initialize it.
-  if (!render_frame_host->render_view_host()->IsRenderViewLive()) {
+  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())) {
+        render_frame_host_->GetSiteInstance());
+    if (!InitRenderView(render_frame_host_->render_view_host(), opener_route_id,
+                        MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame())) {
       return NULL;
     }
   }
-  return render_frame_host;
+  return render_frame_host_.get();
+}
+
+// PlzNavigate
+void RenderFrameHostManager::CleanUpNavigation() {
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  if (speculative_render_frame_host_) {
+    speculative_render_frame_host_->GetProcess()->RemovePendingView();
+    DiscardUnusedFrame(speculative_render_frame_host_.Pass());
+  }
+  if (speculative_web_ui_)
+    speculative_web_ui_.reset();
+  should_reuse_web_ui_ = false;
 }
 
 void RenderFrameHostManager::Observe(
     int type,
     const NotificationSource& source,
     const NotificationDetails& details) {
   switch (type) {
     case NOTIFICATION_RENDERER_PROCESS_CLOSED:
     case NOTIFICATION_RENDERER_PROCESS_CLOSING:
       RendererProcessClosing(
@@ skipping 442 matching lines @@
   // 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 156 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 = 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;
   }
 
   // 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());
+  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();
 
   // 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 333 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