PlzNavigate: make content unit tests work with browser side navigation

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 263 matching lines @@
   // in progress.  Sanity check this for http://crbug.com/276333.
   CHECK(pending_render_frame_host_);
 
   // Unload handlers run in the background, so we should never get an
   // unresponsiveness warning for them.
   CHECK(!render_frame_host_->IsWaitingForUnloadACK());
 
   // If the tab becomes unresponsive during beforeunload while doing a
   // cross-site navigation, proceed with the navigation.  (This assumes that
   // the pending RenderFrameHost is still responsive.)
-  if (render_frame_host_->is_waiting_for_beforeunload_ack()) {
+  if (render_frame_host_->IsWaitingForBeforeUnloadACK()) {
     // Haven't gotten around to starting the request, because we're still
     // waiting for the beforeunload handler to finish.  We'll pretend that it
     // did finish, to let the navigation proceed.  Note that there's a danger
     // that the beforeunload handler will later finish and possibly return
     // false (meaning the navigation should not proceed), but we'll ignore it
     // in this case because it took too long.
     if (pending_render_frame_host_->are_navigations_suspended()) {
       pending_render_frame_host_->SetNavigationsSuspended(
           false, base::TimeTicks::Now());
     }
@@ skipping 158 matching lines @@
 
     // 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
       CancelPending();
       cross_navigation_pending_ = false;
     }
   } else {
@@ skipping 256 matching lines @@
   if (!navigation_rfh->IsRenderFrameLive()) {
     // Recreate the opener chain.
     int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager(
         navigation_rfh->GetSiteInstance());
     if (!InitRenderView(navigation_rfh->render_view_host(), opener_route_id,
                         MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame())) {
       return nullptr;
     }
   }
 
+  cross_navigation_pending_ = navigation_rfh != render_frame_host_.get();

[nasko, 2015/01/21 17:21:33]

nit: If cross_navigation_pending_ is not used for PlzNavigate, why is it set
here then? This method is only used in PlzNavigate, right?

[clamy, 2015/01/21 19:39:41]

Yes, but it is checked by a certain number of tests that fail if we don't
properly set it.

[nasko, 2015/01/21 19:47:06]

Then the comment that it isn't used in PlzNavigate is incorrect. Let's fix that
at some point.

   return navigation_rfh;
 }
 
 // PlzNavigate
 void RenderFrameHostManager::CleanUpNavigation() {
   if (speculative_render_frame_host_)
     DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());
 }
 
 // PlzNavigate
@@ skipping 704 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());
+  cross_navigation_pending_ = false;

[nasko, 2015/01/21 17:21:33]

nit: I'd move this down to where we actually swap the RFHs. It would avoid a
case where a method call between here and the actual swap consults this value,
yet we are in the middle of committing.

[clamy, 2015/01/21 19:39:41]

Done.

   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();
 
   if (!browser_side_navigation) {
@@ skipping 394 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