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 262 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 131 matching lines @@
   render_frame_host_->ClearPendingTransitionRequestData();
 }
 
 void RenderFrameHostManager::DidNavigateFrame(
     RenderFrameHostImpl* render_frame_host,
     bool was_caused_by_user_gesture) {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableBrowserSideNavigation)) {
     if (render_frame_host == speculative_render_frame_host_.get()) {
       CommitPending();
+      cross_navigation_pending_ = false;

[nasko, 2015/01/21 00:41:32]

Shouldn't this flag be cleared in CommitPending itself?

[clamy, 2015/01/21 13:22:11]

Done.

     } else if (render_frame_host == render_frame_host_.get()) {
       // TODO(carlosk): this code doesn't properly handle in-page navigation or
       // interwoven navigation requests.
       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;
@@ skipping 287 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 00:41:32]

Are we sure the returned RFH will actually start off a navigation?

[clamy, 2015/01/21 13:22:11]

No, this is also called by BeginNavigation, so at this point we may not be using
it. However the cross_navigation_pending_ flag is only useful for tests, so I
think it should be ok to set it here.

If we want to make sure that it is only set when a navigation will take place in
the speculative renderer we could have a boolean in the function arguments to
distinguish between when the function is called at BeginNavigation or commit
time. Or have a setter for cross__navigation_pending_ called by
NavigatorImpl::CommitNavigation if needed.

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