Revert of Revert of [site isolation] cross-site transfers should track the...

Revert of Revert of [site isolation] cross-site transfers should track the RenderFrameHost, not the View (patchset #1 of https://codereview.chromium.org/462083003/)

Reason for revert:
The CL turns out to be innocent. Reverting the revert

Original issue's description:
> Revert of [site isolation] cross-site transfers should track the RenderFrameHost, not the View (https://codereview.chromium.org/457003002/)
> 
> Reason for revert:
> Speculatively reverting CL in case it is causing WebNavigationApiTest.OpenTab to fail http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20Chromium&testType=browser_tests&tests=WebNavigationApiTest.OpenTab
> 
> Original issue's description:
> > site isolation: cross-site transfers should track the RenderFrameHost, not the View
> > 
> > This fixes some issues that came up while debugging bug 400850, which inadvertently caused OOPIF-like view reuse in RenderFrameHostManagerTest.AllowTargetedNavigationsAfterSwap.
> > 
> > 1. Make navigation_suspended state be per-RenderFrameHost. What was happening, was that we'd suspend navigations on the non-main frame of the view, and upon resume, we'd switch frames.
> > 
> > 2. Make the map of pending cross site requests be a map of RenderFrameHost IDs instead of RenderViewHost IDs, instead of just assuming that it's the main frame of the view that's navigating.
> > 
> > 3. Add a pending_main_rfh() to the test apparatus, for use by unittests that query the navigation_suspended state.
> > 
> > BUG=304341, 400850
> > 
> > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288867
> 
> TBR=creis@chromium.org,nick@chromium.org
> NOTREECHECKS=true
> NOTRY=true
> BUG=304341, 400850
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289049

TBR=creis@chromium.org,nick@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=304341, 400850

Review URL: https://codereview.chromium.org/466933003

Cr-Commit-Position: refs/heads/master@{#289082}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289082 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/cross_site_request_manager.cc

@@ -9,24 +9,24 @@
 namespace content {
 
 bool CrossSiteRequestManager::HasPendingCrossSiteRequest(int renderer_id,
-                                                         int render_view_id) {
+                                                         int render_frame_id) {
   base::AutoLock lock(lock_);
 
-  std::pair<int, int> key(renderer_id, render_view_id);
-  return pending_cross_site_views_.find(key) !=
-      pending_cross_site_views_.end();
+  std::pair<int, int> key(renderer_id, render_frame_id);
+  return pending_cross_site_frames_.find(key) !=
+         pending_cross_site_frames_.end();
 }
 
 void CrossSiteRequestManager::SetHasPendingCrossSiteRequest(int renderer_id,
-                                                            int render_view_id,
+                                                            int render_frame_id,
                                                             bool has_pending) {
   base::AutoLock lock(lock_);
 
-  std::pair<int, int> key(renderer_id, render_view_id);
+  std::pair<int, int> key(renderer_id, render_frame_id);
   if (has_pending) {
-    pending_cross_site_views_.insert(key);
+    pending_cross_site_frames_.insert(key);
   } else {
-    pending_cross_site_views_.erase(key);
+    pending_cross_site_frames_.erase(key);
   }
 }
 

content/browser/cross_site_request_manager.h

@@ -17,7 +17,7 @@ namespace content {
 
 // CrossSiteRequestManager is used to handle bookkeeping for cross-site
 // requests and responses between the UI and IO threads.  Such requests involve
-// a transition from one RenderViewHost to another within WebContentsImpl, and
+// a transition from one RenderFrameHost to another within WebContentsImpl, and
 // involve coordination with ResourceDispatcherHost.
 //
 // CrossSiteRequestManager is a singleton that may be used on any thread.
@@ -27,22 +27,22 @@ class CrossSiteRequestManager {
   // Returns the singleton instance.
   static CrossSiteRequestManager* GetInstance();
 
-  // Returns whether the RenderViewHost specified by the given IDs currently
+  // Returns whether the RenderFrameHost specified by the given IDs currently
   // has a pending cross-site request.  If so, we will have to delay the
-  // response until the previous RenderViewHost runs its onunload handler.
-  // Called by ResourceDispatcherHost on the IO thread and RenderViewHost on
+  // response until the previous RenderFrameHost runs its onunload handler.
+  // Called by ResourceDispatcherHost on the IO thread and RenderFrameHost on
   // the UI thread.
-  bool HasPendingCrossSiteRequest(int renderer_id, int render_view_id);
+  bool HasPendingCrossSiteRequest(int renderer_id, int render_frame_id);
 
-  // Sets whether the RenderViewHost specified by the given IDs currently has a
-  // pending cross-site request.  Called by RenderViewHost on the UI thread.
+  // Sets whether the RenderFrameHost specified by the given IDs currently has a
+  // pending cross-site request.  Called by RenderFrameHost on the UI thread.
   void SetHasPendingCrossSiteRequest(int renderer_id,
-                                     int render_view_id,
+                                     int render_frame_id,
                                      bool has_pending);
 
  private:
   friend struct DefaultSingletonTraits<CrossSiteRequestManager>;
-  typedef std::set<std::pair<int, int> > RenderViewSet;
+  typedef std::set<std::pair<int, int> > RenderFrameSet;
 
   CrossSiteRequestManager();
   ~CrossSiteRequestManager();
@@ -51,10 +51,10 @@ class CrossSiteRequestManager {
   // class.  You must not block while holding this lock.
   base::Lock lock_;
 
-  // Set of (render_process_host_id, render_view_id) pairs of all
-  // RenderViewHosts that have pending cross-site requests.  Used to pass
-  // information about the RenderViewHosts between the UI and IO threads.
-  RenderViewSet pending_cross_site_views_;
+  // Set of (render_process_host_id, render_frame_id) pairs of all
+  // RenderFrameHosts that have pending cross-site requests.  Used to pass
+  // information about the RenderFrameHosts between the UI and IO threads.
+  RenderFrameSet pending_cross_site_frames_;
 
   DISALLOW_COPY_AND_ASSIGN(CrossSiteRequestManager);
 };

content/browser/frame_host/render_frame_host_impl.cc

@@ -13,6 +13,7 @@
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/accessibility/browser_accessibility_state_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/cross_site_request_manager.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ -166,6 +167,7 @@ RenderFrameHostImpl::RenderFrameHostImpl(RenderViewHostImpl* render_view_host,
       routing_id_(routing_id),
       is_swapped_out_(is_swapped_out),
       renderer_initialized_(false),
+      navigations_suspended_(false),
       weak_ptr_factory_(this) {
   frame_tree_->RegisterRenderFrameHost(this);
   GetProcess()->AddRoute(routing_id_, this);
@@ -188,6 +190,10 @@ RenderFrameHostImpl::~RenderFrameHostImpl() {
   GetProcess()->RemoveRoute(routing_id_);
   g_routing_id_frame_map.Get().erase(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_));
+  // Clean up any leftover state from cross-site requests.
+  CrossSiteRequestManager::GetInstance()->SetHasPendingCrossSiteRequest(
+      GetProcess()->GetID(), routing_id_, false);
+
   if (delegate_)
     delegate_->RenderFrameDeleted(this);
 
@@ -1011,14 +1017,13 @@ void RenderFrameHostImpl::Navigate(const FrameMsg_Navigate_Params& params) {
 
   // Only send the message if we aren't suspended at the start of a cross-site
   // request.
-  if (render_view_host_->navigations_suspended_) {
+  if (navigations_suspended_) {
     // Shouldn't be possible to have a second navigation while suspended, since
     // navigations will only be suspended during a cross-site request.  If a
     // second navigation occurs, RenderFrameHostManager will cancel this pending
     // RFH and create a new pending RFH.
-    DCHECK(!render_view_host_->suspended_nav_params_.get());
-    render_view_host_->suspended_nav_params_.reset(
-        new FrameMsg_Navigate_Params(params));
+    DCHECK(!suspended_nav_params_.get());
+    suspended_nav_params_.reset(new FrameMsg_Navigate_Params(params));
   } else {
     // Get back to a clean state, in case we start a new navigation without
     // completing a RVH swap or unload handler.
@@ -1141,6 +1146,17 @@ void RenderFrameHostImpl::NotificationClosed(int notification_id) {
   cancel_notification_callbacks_.erase(notification_id);
 }
 
+bool RenderFrameHostImpl::HasPendingCrossSiteRequest() {
+  return CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
+      GetProcess()->GetID(), routing_id_);
+}
+
+void RenderFrameHostImpl::SetHasPendingCrossSiteRequest(
+    bool has_pending_request) {
+  CrossSiteRequestManager::GetInstance()->SetHasPendingCrossSiteRequest(
+      GetProcess()->GetID(), routing_id_, has_pending_request);
+}
+
 void RenderFrameHostImpl::PlatformNotificationPermissionRequestDone(
     int request_id, blink::WebNotificationPermission permission) {
   Send(new PlatformNotificationMsg_PermissionRequestComplete(
@@ -1198,4 +1214,31 @@ void RenderFrameHostImpl::ClearPendingTransitionRequestData() {
           routing_id_));
 }
 
+void RenderFrameHostImpl::SetNavigationsSuspended(
+    bool suspend,
+    const base::TimeTicks& proceed_time) {
+  // This should only be called to toggle the state.
+  DCHECK(navigations_suspended_ != suspend);
+
+  navigations_suspended_ = suspend;
+  if (!suspend && suspended_nav_params_) {
+    // There's navigation message params waiting to be sent. Now that we're not
+    // suspended anymore, resume navigation by sending them. If we were swapped
+    // out, we should also stop filtering out the IPC messages now.
+    render_view_host_->SetState(RenderViewHostImpl::STATE_DEFAULT);
+
+    DCHECK(!proceed_time.is_null());
+    suspended_nav_params_->browser_navigation_start = proceed_time;
+    Send(new FrameMsg_Navigate(routing_id_, *suspended_nav_params_));
+    suspended_nav_params_.reset();
+  }
+}
+
+void RenderFrameHostImpl::CancelSuspendedNavigations() {
+  // Clear any state if a pending navigation is canceled or preempted.
+  if (suspended_nav_params_)
+    suspended_nav_params_.reset();
+  navigations_suspended_ = false;
+}
+
 }  // namespace content

content/browser/frame_host/render_frame_host_impl.h

@@ -189,6 +189,30 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // Load the specified URL; this is a shortcut for Navigate().
   void NavigateToURL(const GURL& url);
 
+  // Returns whether navigation messages are currently suspended for this
+  // RenderFrameHost. Only true during a cross-site navigation, while waiting
+  // for the onbeforeunload handler.
+  bool are_navigations_suspended() const { return navigations_suspended_; }
+
+  // Suspends (or unsuspends) any navigation messages from being sent from this
+  // RenderFrameHost. This is called when a pending RenderFrameHost is created
+  // for a cross-site navigation, because we must suspend any navigations until
+  // we hear back from the old renderer's onbeforeunload handler. Note that it
+  // is important that only one navigation event happen after calling this
+  // method with |suspend| equal to true. If |suspend| is false and there is a
+  // suspended_nav_message_, this will send the message. This function should
+  // only be called to toggle the state; callers should check
+  // are_navigations_suspended() first. If |suspend| is false, the time that the
+  // user decided the navigation should proceed should be passed as
+  // |proceed_time|.
+  void SetNavigationsSuspended(bool suspend,
+                               const base::TimeTicks& proceed_time);
+
+  // Clears any suspended navigation state after a cross-site navigation is
+  // canceled or suspended. This is important if we later return to this
+  // RenderFrameHost.
+  void CancelSuspendedNavigations();
+
   // Runs the beforeunload handler for this frame. |for_cross_site_transition|
   // indicates whether this call is for the current frame during a cross-process
   // navigation. False means we're closing the entire tab.
@@ -243,6 +267,17 @@ class CONTENT_EXPORT RenderFrameHostImpl
   gfx::NativeViewAccessible GetParentNativeViewAccessible() const;
 #endif
 
+  // Returns whether this RenderFrameHost has an outstanding cross-site request.
+  // Cleared when we hear the response and start to swap out the old
+  // RenderFrameHost, or if we hear a commit here without a network request.
+  bool HasPendingCrossSiteRequest();
+
+  // Sets whether this RenderFrameHost has an outstanding cross-site request,
+  // for which another renderer will need to run an onunload event handler.
+  // This is called before the first navigation event for this RenderFrameHost,
+  // and cleared when we hear the response or commit.
+  void SetHasPendingCrossSiteRequest(bool has_pending_request);
+
  protected:
   friend class RenderFrameHostFactory;
 
@@ -374,6 +409,19 @@ class CONTENT_EXPORT RenderFrameHostImpl
   bool is_swapped_out_;
   bool renderer_initialized_;
 
+  // Whether we should buffer outgoing Navigate messages rather than sending
+  // them. This will be true when a RenderFrameHost is created for a cross-site
+  // request, until we hear back from the onbeforeunload handler of the old
+  // RenderFrameHost.
+  bool navigations_suspended_;
+
+  // We only buffer the params for a suspended navigation while this RFH is the
+  // pending RenderFrameHost of a RenderFrameHostManager. There will only ever
+  // be one suspended navigation, because RenderFrameHostManager will destroy
+  // the pending RenderFrameHost and create a new one if a second navigation
+  // occurs.
+  scoped_ptr<FrameMsg_Navigate_Params> suspended_nav_params_;
+
   // When the last BeforeUnload message was sent.
   base::TimeTicks send_before_unload_start_time_;
 

content/browser/frame_host/render_frame_host_manager.cc

@@ -273,9 +273,8 @@ bool RenderFrameHostManager::ShouldCloseTabOnUnresponsiveRenderer() {
     // 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_->render_view_host()->
-            are_navigations_suspended()) {
-      pending_render_frame_host_->render_view_host()->SetNavigationsSuspended(
+    if (pending_render_frame_host_->are_navigations_suspended()) {
+      pending_render_frame_host_->SetNavigationsSuspended(
           false, base::TimeTicks::Now());
     }
   }
@@ -298,10 +297,9 @@ void RenderFrameHostManager::OnBeforeUnloadACK(
       // already made by ShouldCloseTabOnUnresponsiveRenderer.  In that case, it
       // is ok to do nothing here.
       if (pending_render_frame_host_ &&
-          pending_render_frame_host_->render_view_host()->
-              are_navigations_suspended()) {
-        pending_render_frame_host_->render_view_host()->
-            SetNavigationsSuspended(false, proceed_time);
+          pending_render_frame_host_->are_navigations_suspended()) {
+        pending_render_frame_host_->SetNavigationsSuspended(false,
+                                                            proceed_time);
       }
     } else {
       // Current page says to cancel.
@@ -458,8 +456,7 @@ void RenderFrameHostManager::DidNavigateFrame(
     // then we still need to swap out the old RFH first and run its unload
     // handler, only if it hasn't happened yet.  OK for that to happen in the
     // background.
-    if (pending_render_frame_host_->render_view_host()->
-            HasPendingCrossSiteRequest() &&
+    if (pending_render_frame_host_->HasPendingCrossSiteRequest() &&
         pending_render_frame_host_->render_view_host()->rvh_state() ==
             RenderViewHostImpl::STATE_DEFAULT) {
       SwapOutOldPage();
@@ -549,8 +546,7 @@ void RenderFrameHostManager::SwapOutOldPage() {
   // navigation.  Thus, we no longer need to remember that the RenderFrameHost
   // is part of a pending cross-site request.
   if (pending_render_frame_host_) {
-    pending_render_frame_host_->render_view_host()->
-        SetHasPendingCrossSiteRequest(false);
+    pending_render_frame_host_->SetHasPendingCrossSiteRequest(false);
   }
 }
 
@@ -1409,8 +1405,7 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
     // 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_->render_view_host()->
-               are_navigations_suspended());
+    DCHECK(!pending_render_frame_host_->are_navigations_suspended());
     bool is_transfer =
         entry.transferred_global_request_id() != GlobalRequestID();
     if (is_transfer) {
@@ -1425,15 +1420,13 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
       render_frame_host_->render_view_host()->Send(new ViewMsg_Stop(
           render_frame_host_->render_view_host()->GetRoutingID()));
 
-      pending_render_frame_host_->render_view_host()->SetNavigationsSuspended(
-          true, base::TimeTicks());
+      pending_render_frame_host_->SetNavigationsSuspended(true,
+                                                          base::TimeTicks());
 
-      // Tell the CrossSiteRequestManager that this RVH has a pending cross-site
+      // Tell the CrossSiteRequestManager that this RFH has a pending cross-site
       // request, so that ResourceDispatcherHost will know to tell us to run the
       // old page's unload handler before it sends the response.
-      // TODO(creis): This needs to be on the RFH.
-      pending_render_frame_host_->render_view_host()->
-          SetHasPendingCrossSiteRequest(true);
+      pending_render_frame_host_->SetHasPendingCrossSiteRequest(true);
     }
 
     // We now have a pending RFH.
@@ -1508,7 +1501,7 @@ void RenderFrameHostManager::CancelPending() {
       pending_render_frame_host->GetSiteInstance());
   if (site_instance->active_view_count() > 1) {
     // Any currently suspended navigations are no longer needed.
-    pending_render_frame_host->render_view_host()->CancelSuspendedNavigations();
+    pending_render_frame_host->CancelSuspendedNavigations();
 
     RenderFrameProxyHost* proxy =
         new RenderFrameProxyHost(site_instance, frame_tree_node_);

content/browser/frame_host/render_frame_host_manager_unittest.cc

@@ -925,7 +925,7 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) {
 
   // Check that the navigation is still suspended because the old RVH
   // is not swapped out, yet.
-  EXPECT_TRUE(host2->render_view_host()->are_navigations_suspended());
+  EXPECT_TRUE(host2->are_navigations_suspended());
   MockRenderProcessHost* test_process_host2 =
       static_cast<MockRenderProcessHost*>(host2->GetProcess());
   test_process_host2->sink().ClearMessages();
@@ -976,8 +976,7 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) {
   EXPECT_NE(host3->GetProcess()->GetID(), host2_process_id);
 
   // Navigations in the new RVH should be suspended.
-  EXPECT_TRUE(static_cast<RenderViewHostImpl*>(
-      host3->render_view_host())->are_navigations_suspended());
+  EXPECT_TRUE(host3->are_navigations_suspended());
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(manager->current_frame_host()->is_swapped_out());
 
@@ -1048,10 +1047,10 @@ TEST_F(RenderFrameHostManagerTest, NewCrossNavigationBetweenSwapOutAndCommit) {
   // Pending rvh2 is already deleted.
   contents()->ProceedWithCrossSiteNavigation();
 
-  TestRenderViewHost* rvh3 = pending_test_rvh();
-  EXPECT_TRUE(rvh3);
+  TestRenderFrameHost* rfh3 = pending_main_test_rfh();
+  EXPECT_TRUE(rfh3);
   // Navigation should be already unblocked by rvh1.
-  EXPECT_FALSE(rvh3->are_navigations_suspended());
+  EXPECT_FALSE(rfh3->are_navigations_suspended());
 }
 
 // Tests WebUI creation.

content/browser/loader/cross_site_resource_handler.cc

@@ -201,9 +201,10 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
     return DeferForNavigationPolicyCheck(info, response, defer);
   }
 
-  bool swap_needed = should_transfer ||
-      CrossSiteRequestManager::GetInstance()->
-          HasPendingCrossSiteRequest(info->GetChildID(), info->GetRouteID());
+  bool swap_needed =
+      should_transfer ||
+      CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
+          info->GetChildID(), info->GetRenderFrameID());
 
   // If this is a download, just pass the response through without doing a
   // cross-site check.  The renderer will see it is a download and abort the
@@ -293,7 +294,7 @@ void CrossSiteResourceHandler::OnResponseCompleted(
     if (has_started_response_ ||
         status.status() != net::URLRequestStatus::FAILED ||
         !CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
-            info->GetChildID(), info->GetRouteID())) {
+            info->GetChildID(), info->GetRenderFrameID())) {
       next_handler_->OnResponseCompleted(status, security_info, defer);
       return;
     }

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -27,7 +27,6 @@
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/cert_store_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
-#include "content/browser/cross_site_request_manager.h"
 #include "content/browser/download/download_resource_handler.h"
 #include "content/browser/download/save_file_manager.h"
 #include "content/browser/download/save_file_resource_handler.h"

content/browser/renderer_host/render_view_host_impl.cc

@@ -24,7 +24,6 @@
 #include "base/values.h"
 #include "cc/base/switches.h"
 #include "content/browser/child_process_security_policy_impl.h"
-#include "content/browser/cross_site_request_manager.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/gpu/compositor_util.h"
@@ -189,7 +188,6 @@ RenderViewHostImpl::RenderViewHostImpl(
       instance_(static_cast<SiteInstanceImpl*>(instance)),
       waiting_for_drag_context_response_(false),
       enabled_bindings_(0),
-      navigations_suspended_(false),
       main_frame_routing_id_(main_frame_routing_id),
       run_modal_reply_msg_(NULL),
       run_modal_opener_id_(MSG_ROUTING_NONE),
@@ -240,10 +238,6 @@ RenderViewHostImpl::~RenderViewHostImpl() {
 
   delegate_->RenderViewDeleted(this);
 
-  // Be sure to clean up any leftover state from cross-site requests.
-  CrossSiteRequestManager::GetInstance()->SetHasPendingCrossSiteRequest(
-      GetProcess()->GetID(), GetRoutingID(), false);
-
   // If this was swapped out, it already decremented the active view
   // count of the SiteInstance it belongs to.
   if (IsRVHStateActive(rvh_state_))
@@ -500,34 +494,6 @@ void RenderViewHostImpl::NavigateToURL(const GURL& url) {
   delegate_->GetFrameTree()->GetMainFrame()->NavigateToURL(url);
 }
 
-void RenderViewHostImpl::SetNavigationsSuspended(
-    bool suspend,
-    const base::TimeTicks& proceed_time) {
-  // This should only be called to toggle the state.
-  DCHECK(navigations_suspended_ != suspend);
-
-  navigations_suspended_ = suspend;
-  if (!suspend && suspended_nav_params_) {
-    // There's navigation message params waiting to be sent.  Now that we're not
-    // suspended anymore, resume navigation by sending them.  If we were swapped
-    // out, we should also stop filtering out the IPC messages now.
-    SetState(STATE_DEFAULT);
-
-    DCHECK(!proceed_time.is_null());
-    suspended_nav_params_->browser_navigation_start = proceed_time;
-    Send(new FrameMsg_Navigate(
-        main_frame_routing_id_, *suspended_nav_params_.get()));
-    suspended_nav_params_.reset();
-  }
-}
-
-void RenderViewHostImpl::CancelSuspendedNavigations() {
-  // Clear any state if a pending navigation is canceled or pre-empted.
-  if (suspended_nav_params_)
-    suspended_nav_params_.reset();
-  navigations_suspended_ = false;
-}
-
 void RenderViewHostImpl::SuppressDialogsUntilSwapOut() {
   Send(new ViewMsg_SuppressDialogsUntilSwapOut(GetRoutingID()));
 }
@@ -648,17 +614,6 @@ void RenderViewHostImpl::ClosePageIgnoringUnloadEvents() {
   delegate_->Close(this);
 }
 
-bool RenderViewHostImpl::HasPendingCrossSiteRequest() {
-  return CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
-      GetProcess()->GetID(), GetRoutingID());
-}
-
-void RenderViewHostImpl::SetHasPendingCrossSiteRequest(
-    bool has_pending_request) {
-  CrossSiteRequestManager::GetInstance()->SetHasPendingCrossSiteRequest(
-      GetProcess()->GetID(), GetRoutingID(), has_pending_request);
-}
-
 #if defined(OS_ANDROID)
 void RenderViewHostImpl::ActivateNearestFindResult(int request_id,
                                                    float x,

content/browser/renderer_host/render_view_host_impl.h

@@ -260,30 +260,6 @@ class CONTENT_EXPORT RenderViewHostImpl
   // RenderFrameHostImpl.
   void NavigateToURL(const GURL& url);
 
-  // Returns whether navigation messages are currently suspended for this
-  // RenderViewHost.  Only true during a cross-site navigation, while waiting
-  // for the onbeforeunload handler.
-  bool are_navigations_suspended() const { return navigations_suspended_; }
-
-  // Suspends (or unsuspends) any navigation messages from being sent from this
-  // RenderViewHost.  This is called when a pending RenderViewHost is created
-  // for a cross-site navigation, because we must suspend any navigations until
-  // we hear back from the old renderer's onbeforeunload handler.  Note that it
-  // is important that only one navigation event happen after calling this
-  // method with |suspend| equal to true.  If |suspend| is false and there is
-  // a suspended_nav_message_, this will send the message.  This function
-  // should only be called to toggle the state; callers should check
-  // are_navigations_suspended() first. If |suspend| is false, the time that the
-  // user decided the navigation should proceed should be passed as
-  // |proceed_time|.
-  void SetNavigationsSuspended(bool suspend,
-                               const base::TimeTicks& proceed_time);
-
-  // Clears any suspended navigation state after a cross-site navigation is
-  // canceled or suspended.  This is important if we later return to this
-  // RenderViewHost.
-  void CancelSuspendedNavigations();
-
   // Whether this RenderViewHost has been swapped out to be displayed by a
   // different process.
   bool IsSwappedOut() const { return rvh_state_ == STATE_SWAPPED_OUT; }
@@ -316,17 +292,6 @@ class CONTENT_EXPORT RenderViewHostImpl
   // and the user has agreed to continue with closing the page.
   void ClosePageIgnoringUnloadEvents();
 
-  // Returns whether this RenderViewHost has an outstanding cross-site request.
-  // Cleared when we hear the response and start to swap out the old
-  // RenderViewHost, or if we hear a commit here without a network request.
-  bool HasPendingCrossSiteRequest();
-
-  // Sets whether this RenderViewHost has an outstanding cross-site request,
-  // for which another renderer will need to run an onunload event handler.
-  // This is called before the first navigation event for this RenderViewHost,
-  // and cleared when we hear the response or commit.
-  void SetHasPendingCrossSiteRequest(bool has_pending_request);
-
   // Tells the renderer view to focus the first (last if reverse is true) node.
   void SetInitialFocus(bool reverse);
 
@@ -534,19 +499,6 @@ class CONTENT_EXPORT RenderViewHostImpl
   // See BindingsPolicy for details.
   int enabled_bindings_;
 
-  // Whether we should buffer outgoing Navigate messages rather than sending
-  // them.  This will be true when a RenderViewHost is created for a cross-site
-  // request, until we hear back from the onbeforeunload handler of the old
-  // RenderViewHost.
-  // TODO(nasko): Move to RenderFrameHost, as this is per-frame state.
-  bool navigations_suspended_;
-
-  // We only buffer the params for a suspended navigation while we have a
-  // pending RVH for a WebContentsImpl.  There will only ever be one suspended
-  // navigation, because WebContentsImpl will destroy the pending RVH and create
-  // a new one if a second navigation occurs.
-  // TODO(nasko): Move to RenderFrameHost, as this is per-frame state.
-  scoped_ptr<FrameMsg_Navigate_Params> suspended_nav_params_;
 
   // The current state of this RVH.
   // TODO(nasko): Move to RenderFrameHost, as this is per-frame state.

content/browser/web_contents/web_contents_impl_unittest.cc

@@ -500,13 +500,12 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) {
       static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
   int pending_rvh_delete_count = 0;
   pending_rvh->set_delete_counter(&pending_rvh_delete_count);
-  RenderFrameHostImpl* pending_rfh = contents()->GetFrameTree()->root()->
-      render_manager()->pending_frame_host();
+  RenderFrameHostImpl* pending_rfh = pending_main_test_rfh();
 
-  // Navigations should be suspended in pending_rvh until BeforeUnloadACK.
-  EXPECT_TRUE(pending_rvh->are_navigations_suspended());
+  // Navigations should be suspended in pending_rfh until BeforeUnloadACK.
+  EXPECT_TRUE(pending_rfh->are_navigations_suspended());
   orig_rvh->SendBeforeUnloadACK(true);
-  EXPECT_FALSE(pending_rvh->are_navigations_suspended());
+  EXPECT_FALSE(pending_rfh->are_navigations_suspended());
 
   // DidNavigate from the pending page
   contents()->TestDidNavigate(
@@ -532,23 +531,22 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) {
 
   // Going back should switch SiteInstances again.  The first SiteInstance is
   // stored in the NavigationEntry, so it should be the same as at the start.
-  // We should use the same RVH as before, swapping it back in.
+  // We should use the same RFH as before, swapping it back in.
   controller().GoBack();
-  TestRenderViewHost* goback_rvh =
-      static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
-  EXPECT_EQ(orig_rvh, goback_rvh);
+  TestRenderFrameHost* goback_rfh = pending_main_test_rfh();
+  EXPECT_EQ(orig_rfh, goback_rfh);
   EXPECT_TRUE(contents()->cross_navigation_pending());
 
-  // Navigations should be suspended in goback_rvh until BeforeUnloadACK.
-  EXPECT_TRUE(goback_rvh->are_navigations_suspended());
+  // Navigations should be suspended in goback_rfh until BeforeUnloadACK.
+  EXPECT_TRUE(goback_rfh->are_navigations_suspended());
   pending_rvh->SendBeforeUnloadACK(true);
-  EXPECT_FALSE(goback_rvh->are_navigations_suspended());
+  EXPECT_FALSE(goback_rfh->are_navigations_suspended());
 
   // DidNavigate from the back action
   contents()->TestDidNavigate(
-      goback_rvh, 1, url2, PAGE_TRANSITION_TYPED);
+      goback_rfh->GetRenderViewHost(), 1, url2, PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->cross_navigation_pending());
-  EXPECT_EQ(goback_rvh, contents()->GetRenderViewHost());
+  EXPECT_EQ(goback_rfh->GetRenderViewHost(), contents()->GetRenderViewHost());
   EXPECT_EQ(instance1, contents()->GetSiteInstance());
   // The pending RFH should now be swapped out, not deleted.
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->
@@ -676,8 +674,7 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) {
   SetBrowserClientForTesting(&browser_client);
 
   TestRenderViewHost* orig_rvh = test_rvh();
-  RenderFrameHostImpl* orig_rfh =
-      contents()->GetFrameTree()->root()->current_frame_host();
+  RenderFrameHostImpl* orig_rfh = main_test_rfh();
   int orig_rvh_delete_count = 0;
   orig_rvh->set_delete_counter(&orig_rvh_delete_count);
   SiteInstanceImpl* orig_instance =
@@ -727,15 +724,17 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) {
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_EQ(url, contents()->GetLastCommittedURL());
   EXPECT_EQ(url2, contents()->GetVisibleURL());
-  TestRenderViewHost* pending_rvh =
-      static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
+  TestRenderFrameHost* pending_rfh = pending_main_test_rfh();
+  TestRenderViewHost* pending_rvh = pending_test_rvh();
+  EXPECT_EQ(pending_rfh->GetRenderViewHost(), pending_rvh);
   int pending_rvh_delete_count = 0;
   pending_rvh->set_delete_counter(&pending_rvh_delete_count);
 
   // Navigations should be suspended in pending_rvh until BeforeUnloadACK.
-  EXPECT_TRUE(pending_rvh->are_navigations_suspended());
+  EXPECT_TRUE(pending_rfh->are_navigations_suspended());
   orig_rvh->SendBeforeUnloadACK(true);
-  EXPECT_FALSE(pending_rvh->are_navigations_suspended());
+  EXPECT_FALSE(pending_rfh->are_navigations_suspended());
+  EXPECT_EQ(pending_rfh->GetRenderViewHost(), pending_rvh);
 
   // DidNavigate from the pending page.
   contents()->TestDidNavigate(

content/public/test/test_renderer_host.cc

@@ -112,9 +112,19 @@ RenderViewHost* RenderViewHostTestHarness::active_rvh() {
 }
 
 RenderFrameHost* RenderViewHostTestHarness::main_rfh() {
-  WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(
-      this->web_contents());
-  return web_contents->GetFrameTree()->GetMainFrame();
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(this->web_contents());
+  RenderFrameHostManager* main_frame_render_manager =
+      web_contents->GetFrameTree()->root()->render_manager();
+  return main_frame_render_manager->current_frame_host();
+}
+
+RenderFrameHost* RenderViewHostTestHarness::pending_main_rfh() {
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(this->web_contents());
+  RenderFrameHostManager* main_frame_render_manager =
+      web_contents->GetFrameTree()->root()->render_manager();
+  return main_frame_render_manager->pending_frame_host();
 }
 
 BrowserContext* RenderViewHostTestHarness::browser_context() {

content/public/test/test_renderer_host.h

@@ -160,6 +160,7 @@ class RenderViewHostTestHarness : public testing::Test {
   RenderViewHost* pending_rvh();
   RenderViewHost* active_rvh();
   RenderFrameHost* main_rfh();
+  RenderFrameHost* pending_main_rfh();
   BrowserContext* browser_context();
   MockRenderProcessHost* process();
 

content/test/test_render_view_host.cc

@@ -389,6 +389,10 @@ TestRenderFrameHost* RenderViewHostImplTestHarness::main_test_rfh() {
   return static_cast<TestRenderFrameHost*>(main_rfh());
 }
 
+TestRenderFrameHost* RenderViewHostImplTestHarness::pending_main_test_rfh() {
+  return static_cast<TestRenderFrameHost*>(pending_main_rfh());
+}
+
 TestWebContents* RenderViewHostImplTestHarness::contents() {
   return static_cast<TestWebContents*>(web_contents());
 }

content/test/test_render_view_host.h

@@ -368,6 +368,7 @@ class RenderViewHostImplTestHarness : public RenderViewHostTestHarness {
   TestRenderViewHost* pending_test_rvh();
   TestRenderViewHost* active_test_rvh();
   TestRenderFrameHost* main_test_rfh();
+  TestRenderFrameHost* pending_main_test_rfh();
   TestWebContents* contents();
 
  private: