summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorpkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-12 19:29:47 +0000
committerpkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-12 19:30:59 +0000
commit4e49497a7ec0da1dbc00c569b7457a221c9214d3 (patch)
treef529169dfa9b317e5b7a9ab91683fe05d2b4ab42
parentc3f5a257d99feea7645d93cd7586200b980290f8 (diff)
downloadchromium_src-4e49497a7ec0da1dbc00c569b7457a221c9214d3.zip
chromium_src-4e49497a7ec0da1dbc00c569b7457a221c9214d3.tar.gz
chromium_src-4e49497a7ec0da1dbc00c569b7457a221c9214d3.tar.bz2
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 Review URL: https://codereview.chromium.org/462083003 Cr-Commit-Position: refs/heads/master@{#289049} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289049 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--content/browser/cross_site_request_manager.cc16
-rw-r--r--content/browser/cross_site_request_manager.h26
-rw-r--r--content/browser/frame_host/render_frame_host_impl.cc51
-rw-r--r--content/browser/frame_host/render_frame_host_impl.h48
-rw-r--r--content/browser/frame_host/render_frame_host_manager.cc33
-rw-r--r--content/browser/frame_host/render_frame_host_manager_unittest.cc11
-rw-r--r--content/browser/loader/cross_site_resource_handler.cc9
-rw-r--r--content/browser/loader/resource_dispatcher_host_impl.cc1
-rw-r--r--content/browser/renderer_host/render_view_host_impl.cc45
-rw-r--r--content/browser/renderer_host/render_view_host_impl.h48
-rw-r--r--content/browser/web_contents/web_contents_impl_unittest.cc39
-rw-r--r--content/public/test/test_renderer_host.cc16
-rw-r--r--content/public/test/test_renderer_host.h1
-rw-r--r--content/test/test_render_view_host.cc4
-rw-r--r--content/test/test_render_view_host.h1
15 files changed, 172 insertions, 177 deletions
diff --git a/content/browser/cross_site_request_manager.cc b/content/browser/cross_site_request_manager.cc
index 5ce71bf..2fa38b1 100644
--- a/content/browser/cross_site_request_manager.cc
+++ b/content/browser/cross_site_request_manager.cc
@@ -9,24 +9,24 @@
namespace content {
bool CrossSiteRequestManager::HasPendingCrossSiteRequest(int renderer_id,
- int render_frame_id) {
+ int render_view_id) {
base::AutoLock lock(lock_);
- std::pair<int, int> key(renderer_id, render_frame_id);
- return pending_cross_site_frames_.find(key) !=
- pending_cross_site_frames_.end();
+ std::pair<int, int> key(renderer_id, render_view_id);
+ return pending_cross_site_views_.find(key) !=
+ pending_cross_site_views_.end();
}
void CrossSiteRequestManager::SetHasPendingCrossSiteRequest(int renderer_id,
- int render_frame_id,
+ int render_view_id,
bool has_pending) {
base::AutoLock lock(lock_);
- std::pair<int, int> key(renderer_id, render_frame_id);
+ std::pair<int, int> key(renderer_id, render_view_id);
if (has_pending) {
- pending_cross_site_frames_.insert(key);
+ pending_cross_site_views_.insert(key);
} else {
- pending_cross_site_frames_.erase(key);
+ pending_cross_site_views_.erase(key);
}
}
diff --git a/content/browser/cross_site_request_manager.h b/content/browser/cross_site_request_manager.h
index bf04c8a..29417d7 100644
--- a/content/browser/cross_site_request_manager.h
+++ b/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 RenderFrameHost to another within WebContentsImpl, and
+// a transition from one RenderViewHost 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 RenderFrameHost specified by the given IDs currently
+ // Returns whether the RenderViewHost specified by the given IDs currently
// has a pending cross-site request. If so, we will have to delay the
- // response until the previous RenderFrameHost runs its onunload handler.
- // Called by ResourceDispatcherHost on the IO thread and RenderFrameHost on
+ // response until the previous RenderViewHost runs its onunload handler.
+ // Called by ResourceDispatcherHost on the IO thread and RenderViewHost on
// the UI thread.
- bool HasPendingCrossSiteRequest(int renderer_id, int render_frame_id);
+ bool HasPendingCrossSiteRequest(int renderer_id, int render_view_id);
- // Sets whether the RenderFrameHost specified by the given IDs currently has a
- // pending cross-site request. Called by RenderFrameHost on the UI thread.
+ // Sets whether the RenderViewHost specified by the given IDs currently has a
+ // pending cross-site request. Called by RenderViewHost on the UI thread.
void SetHasPendingCrossSiteRequest(int renderer_id,
- int render_frame_id,
+ int render_view_id,
bool has_pending);
private:
friend struct DefaultSingletonTraits<CrossSiteRequestManager>;
- typedef std::set<std::pair<int, int> > RenderFrameSet;
+ typedef std::set<std::pair<int, int> > RenderViewSet;
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_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_;
+ // 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_;
DISALLOW_COPY_AND_ASSIGN(CrossSiteRequestManager);
};
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 832911c..c9125d8 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -13,7 +13,6 @@
#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"
@@ -167,7 +166,6 @@ 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);
@@ -190,10 +188,6 @@ 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);
@@ -1017,13 +1011,14 @@ 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 (navigations_suspended_) {
+ if (render_view_host_->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(!suspended_nav_params_.get());
- suspended_nav_params_.reset(new FrameMsg_Navigate_Params(params));
+ DCHECK(!render_view_host_->suspended_nav_params_.get());
+ render_view_host_->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.
@@ -1146,17 +1141,6 @@ 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(
@@ -1214,31 +1198,4 @@ 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
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
index de6a58e..cb9f60d 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -189,30 +189,6 @@ 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.
@@ -267,17 +243,6 @@ 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;
@@ -409,19 +374,6 @@ 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_;
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index b40cc2b..618391f 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -273,8 +273,9 @@ 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_->are_navigations_suspended()) {
- pending_render_frame_host_->SetNavigationsSuspended(
+ if (pending_render_frame_host_->render_view_host()->
+ are_navigations_suspended()) {
+ pending_render_frame_host_->render_view_host()->SetNavigationsSuspended(
false, base::TimeTicks::Now());
}
}
@@ -297,9 +298,10 @@ 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_->are_navigations_suspended()) {
- pending_render_frame_host_->SetNavigationsSuspended(false,
- proceed_time);
+ pending_render_frame_host_->render_view_host()->
+ are_navigations_suspended()) {
+ pending_render_frame_host_->render_view_host()->
+ SetNavigationsSuspended(false, proceed_time);
}
} else {
// Current page says to cancel.
@@ -456,7 +458,8 @@ 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_->HasPendingCrossSiteRequest() &&
+ if (pending_render_frame_host_->render_view_host()->
+ HasPendingCrossSiteRequest() &&
pending_render_frame_host_->render_view_host()->rvh_state() ==
RenderViewHostImpl::STATE_DEFAULT) {
SwapOutOldPage();
@@ -546,7 +549,8 @@ 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_->SetHasPendingCrossSiteRequest(false);
+ pending_render_frame_host_->render_view_host()->
+ SetHasPendingCrossSiteRequest(false);
}
}
@@ -1405,7 +1409,8 @@ 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_->are_navigations_suspended());
+ DCHECK(!pending_render_frame_host_->render_view_host()->
+ are_navigations_suspended());
bool is_transfer =
entry.transferred_global_request_id() != GlobalRequestID();
if (is_transfer) {
@@ -1420,13 +1425,15 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
render_frame_host_->render_view_host()->Send(new ViewMsg_Stop(
render_frame_host_->render_view_host()->GetRoutingID()));
- pending_render_frame_host_->SetNavigationsSuspended(true,
- base::TimeTicks());
+ pending_render_frame_host_->render_view_host()->SetNavigationsSuspended(
+ true, base::TimeTicks());
- // Tell the CrossSiteRequestManager that this RFH has a pending cross-site
+ // Tell the CrossSiteRequestManager that this RVH 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.
- pending_render_frame_host_->SetHasPendingCrossSiteRequest(true);
+ // TODO(creis): This needs to be on the RFH.
+ pending_render_frame_host_->render_view_host()->
+ SetHasPendingCrossSiteRequest(true);
}
// We now have a pending RFH.
@@ -1501,7 +1508,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->CancelSuspendedNavigations();
+ pending_render_frame_host->render_view_host()->CancelSuspendedNavigations();
RenderFrameProxyHost* proxy =
new RenderFrameProxyHost(site_instance, frame_tree_node_);
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index e8db0e1..1a76c36 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/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->are_navigations_suspended());
+ EXPECT_TRUE(host2->render_view_host()->are_navigations_suspended());
MockRenderProcessHost* test_process_host2 =
static_cast<MockRenderProcessHost*>(host2->GetProcess());
test_process_host2->sink().ClearMessages();
@@ -976,7 +976,8 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) {
EXPECT_NE(host3->GetProcess()->GetID(), host2_process_id);
// Navigations in the new RVH should be suspended.
- EXPECT_TRUE(host3->are_navigations_suspended());
+ EXPECT_TRUE(static_cast<RenderViewHostImpl*>(
+ host3->render_view_host())->are_navigations_suspended());
EXPECT_EQ(host, manager->current_frame_host());
EXPECT_FALSE(manager->current_frame_host()->is_swapped_out());
@@ -1047,10 +1048,10 @@ TEST_F(RenderFrameHostManagerTest, NewCrossNavigationBetweenSwapOutAndCommit) {
// Pending rvh2 is already deleted.
contents()->ProceedWithCrossSiteNavigation();
- TestRenderFrameHost* rfh3 = pending_main_test_rfh();
- EXPECT_TRUE(rfh3);
+ TestRenderViewHost* rvh3 = pending_test_rvh();
+ EXPECT_TRUE(rvh3);
// Navigation should be already unblocked by rvh1.
- EXPECT_FALSE(rfh3->are_navigations_suspended());
+ EXPECT_FALSE(rvh3->are_navigations_suspended());
}
// Tests WebUI creation.
diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc
index 486f6f9..03589bf 100644
--- a/content/browser/loader/cross_site_resource_handler.cc
+++ b/content/browser/loader/cross_site_resource_handler.cc
@@ -201,10 +201,9 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
return DeferForNavigationPolicyCheck(info, response, defer);
}
- bool swap_needed =
- should_transfer ||
- CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
- info->GetChildID(), info->GetRenderFrameID());
+ bool swap_needed = should_transfer ||
+ CrossSiteRequestManager::GetInstance()->
+ HasPendingCrossSiteRequest(info->GetChildID(), info->GetRouteID());
// 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
@@ -294,7 +293,7 @@ void CrossSiteResourceHandler::OnResponseCompleted(
if (has_started_response_ ||
status.status() != net::URLRequestStatus::FAILED ||
!CrossSiteRequestManager::GetInstance()->HasPendingCrossSiteRequest(
- info->GetChildID(), info->GetRenderFrameID())) {
+ info->GetChildID(), info->GetRouteID())) {
next_handler_->OnResponseCompleted(status, security_info, defer);
return;
}
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index e30a0f6..17cfcf7 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -27,6 +27,7 @@
#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"
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 5006500..84d4b93 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -24,6 +24,7 @@
#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"
@@ -188,6 +189,7 @@ 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),
@@ -238,6 +240,10 @@ 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_))
@@ -494,6 +500,34 @@ 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()));
}
@@ -614,6 +648,17 @@ 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,
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index fa76384..6d69007 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -260,6 +260,30 @@ 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; }
@@ -292,6 +316,17 @@ 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);
@@ -499,6 +534,19 @@ 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.
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index a3e2055..fdee01a 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -500,12 +500,13 @@ 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 = pending_main_test_rfh();
+ RenderFrameHostImpl* pending_rfh = contents()->GetFrameTree()->root()->
+ render_manager()->pending_frame_host();
- // Navigations should be suspended in pending_rfh until BeforeUnloadACK.
- EXPECT_TRUE(pending_rfh->are_navigations_suspended());
+ // Navigations should be suspended in pending_rvh until BeforeUnloadACK.
+ EXPECT_TRUE(pending_rvh->are_navigations_suspended());
orig_rvh->SendBeforeUnloadACK(true);
- EXPECT_FALSE(pending_rfh->are_navigations_suspended());
+ EXPECT_FALSE(pending_rvh->are_navigations_suspended());
// DidNavigate from the pending page
contents()->TestDidNavigate(
@@ -531,22 +532,23 @@ 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 RFH as before, swapping it back in.
+ // We should use the same RVH as before, swapping it back in.
controller().GoBack();
- TestRenderFrameHost* goback_rfh = pending_main_test_rfh();
- EXPECT_EQ(orig_rfh, goback_rfh);
+ TestRenderViewHost* goback_rvh =
+ static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
+ EXPECT_EQ(orig_rvh, goback_rvh);
EXPECT_TRUE(contents()->cross_navigation_pending());
- // Navigations should be suspended in goback_rfh until BeforeUnloadACK.
- EXPECT_TRUE(goback_rfh->are_navigations_suspended());
+ // Navigations should be suspended in goback_rvh until BeforeUnloadACK.
+ EXPECT_TRUE(goback_rvh->are_navigations_suspended());
pending_rvh->SendBeforeUnloadACK(true);
- EXPECT_FALSE(goback_rfh->are_navigations_suspended());
+ EXPECT_FALSE(goback_rvh->are_navigations_suspended());
// DidNavigate from the back action
contents()->TestDidNavigate(
- goback_rfh->GetRenderViewHost(), 1, url2, PAGE_TRANSITION_TYPED);
+ goback_rvh, 1, url2, PAGE_TRANSITION_TYPED);
EXPECT_FALSE(contents()->cross_navigation_pending());
- EXPECT_EQ(goback_rfh->GetRenderViewHost(), contents()->GetRenderViewHost());
+ EXPECT_EQ(goback_rvh, contents()->GetRenderViewHost());
EXPECT_EQ(instance1, contents()->GetSiteInstance());
// The pending RFH should now be swapped out, not deleted.
EXPECT_TRUE(contents()->GetRenderManagerForTesting()->
@@ -674,7 +676,8 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) {
SetBrowserClientForTesting(&browser_client);
TestRenderViewHost* orig_rvh = test_rvh();
- RenderFrameHostImpl* orig_rfh = main_test_rfh();
+ RenderFrameHostImpl* orig_rfh =
+ contents()->GetFrameTree()->root()->current_frame_host();
int orig_rvh_delete_count = 0;
orig_rvh->set_delete_counter(&orig_rvh_delete_count);
SiteInstanceImpl* orig_instance =
@@ -724,17 +727,15 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) {
EXPECT_TRUE(contents()->cross_navigation_pending());
EXPECT_EQ(url, contents()->GetLastCommittedURL());
EXPECT_EQ(url2, contents()->GetVisibleURL());
- TestRenderFrameHost* pending_rfh = pending_main_test_rfh();
- TestRenderViewHost* pending_rvh = pending_test_rvh();
- EXPECT_EQ(pending_rfh->GetRenderViewHost(), pending_rvh);
+ TestRenderViewHost* pending_rvh =
+ static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
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_rfh->are_navigations_suspended());
+ EXPECT_TRUE(pending_rvh->are_navigations_suspended());
orig_rvh->SendBeforeUnloadACK(true);
- EXPECT_FALSE(pending_rfh->are_navigations_suspended());
- EXPECT_EQ(pending_rfh->GetRenderViewHost(), pending_rvh);
+ EXPECT_FALSE(pending_rvh->are_navigations_suspended());
// DidNavigate from the pending page.
contents()->TestDidNavigate(
diff --git a/content/public/test/test_renderer_host.cc b/content/public/test/test_renderer_host.cc
index c9136b7..70d7e66 100644
--- a/content/public/test/test_renderer_host.cc
+++ b/content/public/test/test_renderer_host.cc
@@ -112,19 +112,9 @@ RenderViewHost* RenderViewHostTestHarness::active_rvh() {
}
RenderFrameHost* RenderViewHostTestHarness::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->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();
+ WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(
+ this->web_contents());
+ return web_contents->GetFrameTree()->GetMainFrame();
}
BrowserContext* RenderViewHostTestHarness::browser_context() {
diff --git a/content/public/test/test_renderer_host.h b/content/public/test/test_renderer_host.h
index a3785a9..a514d30 100644
--- a/content/public/test/test_renderer_host.h
+++ b/content/public/test/test_renderer_host.h
@@ -160,7 +160,6 @@ class RenderViewHostTestHarness : public testing::Test {
RenderViewHost* pending_rvh();
RenderViewHost* active_rvh();
RenderFrameHost* main_rfh();
- RenderFrameHost* pending_main_rfh();
BrowserContext* browser_context();
MockRenderProcessHost* process();
diff --git a/content/test/test_render_view_host.cc b/content/test/test_render_view_host.cc
index 5bc9478f..8ee9cf4 100644
--- a/content/test/test_render_view_host.cc
+++ b/content/test/test_render_view_host.cc
@@ -389,10 +389,6 @@ 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());
}
diff --git a/content/test/test_render_view_host.h b/content/test/test_render_view_host.h
index 81ff5d2..5dd8a94 100644
--- a/content/test/test_render_view_host.h
+++ b/content/test/test_render_view_host.h
@@ -368,7 +368,6 @@ class RenderViewHostImplTestHarness : public RenderViewHostTestHarness {
TestRenderViewHost* pending_test_rvh();
TestRenderViewHost* active_test_rvh();
TestRenderFrameHost* main_test_rfh();
- TestRenderFrameHost* pending_main_test_rfh();
TestWebContents* contents();
private: