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 51c9c241ec37466bb2fa073c3c32af10f25c6b61..5c23aa2da20c204d0af7a54b9846fc0dbf414fb7 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -60,7 +60,8 @@ RenderFrameHostManager::RenderFrameHostManager(
       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_);
 }
@@ -69,6 +70,11 @@ RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_)
     UnsetPendingRenderFrameHost();
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    UnsetSpeculativeRenderFrameHost();
+  }
+
   if (render_frame_host_ &&
       render_frame_host_->GetSiteInstance()->active_frame_count() <= 1U) {
     ShutdownRenderFrameProxyHostsInSiteInstance(
@@ -292,6 +298,8 @@ void RenderFrameHostManager::OnBeforeUnloadACK(
     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;
@@ -417,7 +425,26 @@ void RenderFrameHostManager::ClearNavigationTransitionData() {
 
 void RenderFrameHostManager::DidNavigateFrame(
     RenderFrameHostImpl* render_frame_host,
-    bool was_caused_by_user_gesture) {
+    bool was_caused_by_user_gesture,
+    bool was_within_same_page) {
+  DCHECK(render_frame_host);
+  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()) {
+      DCHECK(was_within_same_page || !speculative_render_frame_host_);
+      // TODO(carlosk): if both |was_within_same_page| and
+      // |was_caused_by_user_gesture| are true properly cancel a possible
+      // ongoing navigation request.
+    } 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_) {
     DCHECK(!pending_render_frame_host_);
 
@@ -439,7 +466,7 @@ void RenderFrameHostManager::DidNavigateFrame(
       // 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.
       CancelPending();
       cross_navigation_pending_ = false;
     }
@@ -578,7 +605,12 @@ void RenderFrameHostManager::DiscardUnusedFrame(
     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
+    // 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 {
@@ -623,35 +655,101 @@ void RenderFrameHostManager::ResetProxyHosts() {
 }
 
 // PlzNavigate
-RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
-      const GURL& url,
-      ui::PageTransition transition) {
+void RenderFrameHostManager::BeginNavigation(
+    const CommonNavigationParams& common_params) {
   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.
+  // Cleans up any state in case there's an ongoing navigation.
+  // TODO(carlosk): remove this cleanup here once we properly cancel ongoing
+  // navigations.
+  CleanUpNavigation();
+
+  RenderFrameHostImpl* future_rfh =
+      SelectFrameHostForNavigation(common_params.url, common_params.transition);
+  DCHECK(future_rfh);
+}
 
+// PlzNavigate
+RenderFrameHostImpl* RenderFrameHostManager::SelectFrameHostForNavigation(
+    const GURL& url,
+    ui::PageTransition transition) {
+  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
   // 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);
-
-  // 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()) {
+  RenderFrameHostImpl* rfh_for_navigation = nullptr;
+
+  SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
+  // TODO(carlosk): Replace the default values with the right ones for

[nasko, 2015/01/06 00:03:18]

nit: empty line before the TODO

[carlosk, 2015/01/08 16:05:56]

Done.

+  // 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
+    // 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.
+    CleanUpNavigation();
+    rfh_for_navigation = render_frame_host_.get();
+  } else {
+    // 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()) {
     // 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;
     }
   }

[nasko, 2015/01/06 00:03:18]

Don't we need to initialize the RenderFrame if it isn't live as well?

[carlosk, 2015/01/08 16:05:55]

I'm not sure of what you mean. Is that the renderer? I was assuming that was
what CreateSpeculativeRenderFrameHost (which is based on the existing RFH
creation code) or InitRenderView call chains would do. 

Or are you maybe referring to the currently active RFH?

On this note clamy@ suggested I should add a snippet of code from ::Navigate
that brings the current RFH back to life in case it's currently crashed so that
we show a blank page instead of the sad one (has a comment that refers to Bug
1145340 which we couldn't find anywhere).

[nasko, 2015/01/08 23:36:37]

There are two objects - RenderFrameHost, which is browser side and is created by
CreateSpeculativeRenderFrameHost, and RenderFrame, which is renderer side and is
created by calling InitRenderFrame, which sends an IPC to the renderer.
No, InitRenderView only initializes a RenderView object. You shouldn't depend on
it possibly initializing RenderFrame. CreateSpeculativeRenderFrameHost does
indeed init the RenderFrame by calling CreateRenderFrame, which does it.
No, I was referring to the RFH being created here.
It is a bug from before Chrome was open sourced, so it is in the Google internal
database : ).

[carlosk, 2015/01/09 14:48:52]

Yes, thanks for explaining. That's what I was referring to even though I didn't
phrase it right.
I understand now. So, looking at the current code the only time *ever* where
InitRenderFrame is called is from inside render CreateRenderFrame. As you
mentioned this is already called when creating the speculative RFH. So my
current best guess for your initial question is that no, we shouldn't initialize
the RenderFrame here.

WDYT?
Ack.
 
Ack.

[nasko, 2015/01/09 18:13:07]

No need to do anything extra here.

-  return render_frame_host;
+
+  DCHECK(new_instance->GetProcess()->HasConnection());

[nasko, 2015/01/06 00:03:17]

What's the goal of this DCHECK?

[carlosk, 2015/01/08 16:05:55]

I understand this checks for a connection with an existing renderer process. As
the main reason of existence of a speculative RFH  is to hold a live renderer
for an ongoing navigation this DCHECK seems appropriate.

[nasko, 2015/01/08 23:36:37]

Process creation is asynchronous, so you shouldn't rely that it has completed by
the time you reach this line. This DCHECK has full potential to fire for no
obvious reason.

While the ultimate goal is to optimize process creation, it is the wrong thing
to ask for right here. Processes are an implementation detail of Chrome's choice
how to isolate SiteInstances.

[carlosk, 2015/01/09 14:48:52]

It's a check that already happens in this whole process at
RenderViewHostImpl::CreateRenderView, line 261. We get to that call through:

RFHM::InitRenderView -> WCI::CreateRenderViewForRenderManager ->
RVHI::CreateRenderView

So it doesn't seem like it's going to fire when it shouldn't but as it's already
done it seems OK remove it. WDYT?

This makes me think that the actual creation of the process happens
synchronously but not it's full initialization.

[nasko, 2015/01/09 18:13:07]

If it is already done, then why keep it here? The code in CreateRenderView
explicitly calls GetProcess()->Init(), so it checks that this has succeeded.
 
Process creation is asynchronous, otherwise we would be blocking the UI thread
for the duration of it.
After digging in code, I realized that the HasConnection() only checks
browser-side existence of IPC channel, not that it is actually setup and ready.

Logically, this DCHECK doesn't seem to fit with the spirit of the code in this
method, so please remove it.

[carlosk, 2015/01/12 14:35:25]

Ack and done.

+  return rfh_for_navigation;
+}
+
+// PlzNavigate
+void RenderFrameHostManager::CleanUpNavigation() {
+  scoped_ptr<RenderFrameHostImpl> rfh = UnsetSpeculativeRenderFrameHost();
+  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_) {
+    speculative_render_frame_host_->GetProcess()->RemovePendingView();

[nasko, 2015/01/06 00:03:17]

What codepath does AddPendingView on the speculative RFH?

[carlosk, 2015/01/08 16:05:55]

CreatePendingRenderFrameHost -> CreateRenderFrame

[nasko, 2015/01/08 23:36:37]

This method doesn't touch the pending RFH and we aren't using pending RFH in
PlzNavigate at all.
So my question still stands, where is the AddPendingView on the speculative RFH?

[carlosk, 2015/01/09 14:48:51]

Doh, my bad! I meant to write:

CreateSpeculativeRenderFrameHost -> CreateRenderFrame

Which is the only way a speculative RFH can be created. And CreateRenderFrame
does AddPendingView to the instances it returns.

Sorry for the confusion.

[nasko, 2015/01/09 18:13:07]

Ah, that clarifies it. Thanks.

+    return speculative_render_frame_host_.Pass();
+  }
+  return nullptr;
 }
 
 void RenderFrameHostManager::OnDidStartLoading() {
@@ -1057,7 +1155,7 @@ void RenderFrameHostManager::CreatePendingRenderFrameHost(
   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.
   pending_render_frame_host_ =
       CreateRenderFrame(new_instance, pending_web_ui(), opener_route_id,
                         create_render_frame_flags, nullptr);
@@ -1094,7 +1192,7 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
 
   // 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);
@@ -1112,6 +1210,41 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
   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) {

[nasko, 2015/01/06 00:03:18]

nit: no {} needed for one liners.

[carlosk, 2015/01/08 16:05:55]

Done.

+    return false;
+  }

[nasko, 2015/01/06 00:03:17]

Is there a specific reason why we don't call InitRenderFrame on the new RFH?

[carlosk, 2015/01/08 16:05:55]

That's already done in CreateRenderFrame.

+  speculative_render_frame_host_.reset(new_render_frame_host.release());

[nasko, 2015/01/06 00:03:17]

Why not new_render_frame_host.Pass() instead of new_render_frame_host.release()?

[carlosk, 2015/01/08 16:05:55]

Because there were used with the pending_web_ui and I replicated them.

Switched these two to Pass().

+  speculative_web_ui_.reset(new_web_ui.release());
+  return true;
+}
+
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     SiteInstance* instance,
     WebUIImpl* web_ui,
@@ -1133,8 +1266,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
@@ -1247,11 +1380,19 @@ bool RenderFrameHostManager::InitRenderView(
   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;
+  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.
@@ -1313,29 +1454,40 @@ int RenderFrameHostManager::GetRoutingIdForSiteInstance(
 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;
@@ -1349,10 +1501,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());
+  // 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();
 
@@ -1498,17 +1660,17 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
     // 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.
@@ -1532,14 +1694,6 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
     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