Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1154)

Unified Diff: content/browser/web_contents/render_view_host_manager.cc

Issue 15682009: Eliminate SwapOut message parameters, keeping state in RVHM instead. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Rebase to get Android build fix Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/web_contents/render_view_host_manager.cc
diff --git a/content/browser/web_contents/render_view_host_manager.cc b/content/browser/web_contents/render_view_host_manager.cc
index 3fefe30241ef9c98825750560d2ce6cf831e8f3b..64c16f46858c085e2ab3eddb1c9c016aa3732251 100644
--- a/content/browser/web_contents/render_view_host_manager.cc
+++ b/content/browser/web_contents/render_view_host_manager.cc
@@ -32,6 +32,14 @@
namespace content {
+RenderViewHostManager::PendingNavigationParams::PendingNavigationParams() {
+}
+
+RenderViewHostManager::PendingNavigationParams::PendingNavigationParams(
+ const GlobalRequestID& global_request_id)
+ : global_request_id(global_request_id) {
+}
+
RenderViewHostManager::RenderViewHostManager(
RenderViewHostDelegate* render_view_delegate,
RenderWidgetHostDelegate* render_widget_delegate,
@@ -188,11 +196,17 @@ bool RenderViewHostManager::ShouldCloseTabOnUnresponsiveRenderer() {
if (!cross_navigation_pending_)
return true;
- // If the tab becomes unresponsive during unload while doing a
+ // If the tab becomes unresponsive during {before}unload while doing a
// cross-site navigation, proceed with the navigation. (This assumes that
// the pending RenderViewHost is still responsive.)
- int pending_request_id = pending_render_view_host_->GetPendingRequestId();
- if (pending_request_id == -1) {
+ if (render_view_host_->is_waiting_for_unload_ack()) {
+ // The request has been started and paused while we're waiting for the
+ // unload handler to finish. We'll pretend that it did. The pending
+ // renderer will then be swapped in as part of the usual DidNavigate logic.
+ // (If the unload handler later finishes, this call will be ignored because
+ // the pending_nav_params_ state will already be cleaned up.)
+ current_host()->OnSwappedOut(true);
+ } else if (render_view_host_->is_waiting_for_beforeunload_ack()) {
// 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
@@ -202,24 +216,31 @@ bool RenderViewHostManager::ShouldCloseTabOnUnresponsiveRenderer() {
if (pending_render_view_host_->are_navigations_suspended())
pending_render_view_host_->SetNavigationsSuspended(
false, base::TimeTicks::Now());
- } else {
- // The request has been started and paused while we're waiting for the
- // unload handler to finish. We'll pretend that it did, by notifying the
- // IO thread to let the response continue. The pending renderer will then
- // be swapped in as part of the usual DidNavigate logic. (If the unload
- // handler later finishes, this call will be ignored because the state in
- // CrossSiteResourceHandler will already be cleaned up.)
- ViewMsg_SwapOut_Params params;
- params.closing_process_id = render_view_host_->GetProcess()->GetID();
- params.closing_route_id = render_view_host_->GetRoutingID();
- params.new_render_process_host_id =
- pending_render_view_host_->GetProcess()->GetID();
- params.new_request_id = pending_request_id;
- current_host()->GetProcess()->SimulateSwapOutACK(params);
}
return false;
}
+void RenderViewHostManager::SwappedOut(RenderViewHost* render_view_host) {
+ // Make sure this is from our current RVH, and that we have a pending
+ // navigation from OnCrossSiteResponse. (There may be no pending navigation
+ // for data URLs that don't make network requests, for example.) If not,
+ // just return early and ignore.
+ if (render_view_host != render_view_host_ || !pending_nav_params_.get()) {
+ pending_nav_params_.reset();
+ return;
+ }
+
+ // Now that the unload handler has run, we need to resume the paused response.
+ if (pending_render_view_host_) {
+ RenderProcessHostImpl* pending_process =
+ static_cast<RenderProcessHostImpl*>(
+ pending_render_view_host_->GetProcess());
+ pending_process->ResumeDeferredNavigation(
+ pending_nav_params_->global_request_id);
+ }
+ pending_nav_params_.reset();
+}
+
void RenderViewHostManager::DidNavigateMainFrame(
RenderViewHost* render_view_host) {
if (!cross_navigation_pending_) {
@@ -239,10 +260,8 @@ void RenderViewHostManager::DidNavigateMainFrame(
// If it committed without sending network requests (e.g., data URLs),
// then we still need to swap out the old RVH first and run its unload
// handler. OK for that to happen in the background.
- if (pending_render_view_host_->GetPendingRequestId() == -1) {
- OnCrossSiteResponse(pending_render_view_host_->GetProcess()->GetID(),
- pending_render_view_host_->GetRoutingID());
- }
+ if (pending_render_view_host_->HasPendingCrossSiteRequest())
+ SwapOutOldPage();
CommitPending();
cross_navigation_pending_ = false;
@@ -340,8 +359,20 @@ void RenderViewHostManager::ShouldClosePage(
}
}
-void RenderViewHostManager::OnCrossSiteResponse(int new_render_process_host_id,
- int new_request_id) {
+void RenderViewHostManager::OnCrossSiteResponse(
+ RenderViewHost* pending_render_view_host,
+ const GlobalRequestID& global_request_id) {
+ // This should be called when the pending RVH is ready to commit.
+ DCHECK_EQ(pending_render_view_host_, pending_render_view_host);
+
+ // Remember the request ID until the unload handler has run.
+ pending_nav_params_.reset(new PendingNavigationParams(global_request_id));
+
+ // Run the unload handler of the current page.
+ SwapOutOldPage();
+}
+
+void RenderViewHostManager::SwapOutOldPage() {
// Should only see this while we have a pending renderer.
if (!cross_navigation_pending_)
return;
@@ -350,16 +381,15 @@ void RenderViewHostManager::OnCrossSiteResponse(int new_render_process_host_id,
// Tell the old renderer it is being swapped out. This will fire the unload
// handler (without firing the beforeunload handler a second time). When the
// unload handler finishes and the navigation completes, we will send a
- // message to the ResourceDispatcherHost with the given pending request IDs,
- // allowing the pending RVH's response to resume.
- render_view_host_->SwapOut(new_render_process_host_id, new_request_id);
+ // message to the ResourceDispatcherHost, allowing the pending RVH's response
+ // to resume.
+ render_view_host_->SwapOut();
// ResourceDispatcherHost has told us to run the onunload handler, which
// means it is not a download or unsafe page, and we are going to perform the
// navigation. Thus, we no longer need to remember that the RenderViewHost
// is part of a pending cross-site request.
- pending_render_view_host_->SetHasPendingCrossSiteRequest(false,
- new_request_id);
+ pending_render_view_host_->SetHasPendingCrossSiteRequest(false);
}
void RenderViewHostManager::Observe(
@@ -858,7 +888,7 @@ RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
// 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_view_host_->SetHasPendingCrossSiteRequest(true, -1);
+ pending_render_view_host_->SetHasPendingCrossSiteRequest(true);
// We now have a pending RVH.
DCHECK(!cross_navigation_pending_);
@@ -920,9 +950,7 @@ void RenderViewHostManager::CancelPending() {
// Any currently suspended navigations are no longer needed.
pending_render_view_host->CancelSuspendedNavigations();
- // We can pass -1,-1 because there is no pending response in the
- // ResourceDispatcherHost to unpause.
- pending_render_view_host->SwapOut(-1, -1);
+ pending_render_view_host->SwapOut();
} else {
// We won't be coming back, so shut this one down.
pending_render_view_host->Shutdown();

Powered by Google App Engine
This is Rietveld 408576698