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

Unified Diff: content/browser/frame_host/render_frame_host_manager.cc

Issue 2496233003: Destroy the old RenderWidgetHostView when swapping out a main frame. (Closed)
Patch Set: rebase Created 4 years 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/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 59cbe436c80a945cff6f16a1e98cea75ad0b679d..998ae198d314f60f3a7379871cfb3e4cdc1eefd9 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -699,6 +699,11 @@ void RenderFrameHostManager::DiscardUnusedFrame(
rvh->set_main_frame_routing_id(MSG_ROUTING_NONE);
rvh->set_is_active(false);
rvh->set_is_swapped_out(true);
+
+ if (rvh->GetWidget()->GetView()) {
+ rvh->GetWidget()->GetView()->Destroy();
+ rvh->GetWidget()->SetView(nullptr);
+ }
}
render_frame_host.reset();
@@ -712,6 +717,15 @@ void RenderFrameHostManager::DiscardUnusedFrame(
bool RenderFrameHostManager::DeleteFromPendingList(
RenderFrameHostImpl* render_frame_host) {
+ // If this is a main frame RFH that's about to be deleted, update its RVH's
+ // swapped-out state here. https://crbug.com/505887
+ if (frame_tree_node_->IsMainFrame()) {
+ RenderViewHostImpl* rvh = render_frame_host->render_view_host();
+
+ if (!rvh->is_active())
+ rvh->set_is_swapped_out(true);
+ }
+
for (RFHPendingDeleteList::iterator iter = pending_delete_hosts_.begin();
iter != pending_delete_hosts_.end();
iter++) {
@@ -1759,11 +1773,6 @@ std::unique_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
if (frame_tree_node_->IsMainFrame()) {
success = InitRenderView(render_view_host, proxy);
-
- // If we are reusing the RenderViewHost and it doesn't already have a
- // RenderWidgetHostView, we need to create one if this is the main frame.
- if (!render_view_host->GetWidget()->GetView())
- delegate_->CreateRenderWidgetHostViewForRenderManager(render_view_host);
} else {
DCHECK(render_view_host->IsRenderViewLive());
}
@@ -2164,6 +2173,44 @@ void RenderFrameHostManager::CommitPending() {
// The process will no longer try to exit, so we can decrement the count.
render_frame_host_->GetProcess()->RemovePendingView();
+ // The RenderViewHost keeps track of the main RenderFrameHost routing id.
+ // If this is committing a main frame navigation, update it and set the
+ // routing id in the RenderViewHost associated with the old RenderFrameHost
+ // to MSG_ROUTING_NONE.
+ if (is_main_frame) {
+ RenderViewHostImpl* rvh = render_frame_host_->render_view_host();
+ rvh->set_main_frame_routing_id(render_frame_host_->routing_id());
+
+ // If we are reusing the RenderViewHost, we need to create the
+ // RenderWidgetHostView if this is the main frame.
+ if (rvh->IsRenderViewLive() && !rvh->is_active())
+ delegate_->CreateRenderWidgetHostViewForRenderManager(rvh);
+
+ // If the RenderViewHost is transitioning from swapped out to active state,
+ // it was reused, so dispatch a RenderViewReady event. For example, this
+ // is necessary to hide the sad tab if one is currently displayed. See
+ // https://crbug.com/591984.
+ //
+ // TODO(alexmos): Remove this and move RenderViewReady consumers to use
+ // the main frame's RenderFrameCreated instead.
+ if (!rvh->is_active())
+ rvh->PostRenderViewReady();
+
+ rvh->set_is_active(true);
+ rvh->set_is_swapped_out(false);
+
+ // Tell the old RenderViewHost it is no longer active.
+ RenderViewHostImpl* old_rvh = old_render_frame_host->render_view_host();
+ old_rvh->set_main_frame_routing_id(MSG_ROUTING_NONE);
+ old_rvh->set_is_active(false);
+
+ // Destroy the old RenderWidgetHostView.
+ if (old_rvh->GetWidget()->GetView()) {
+ old_rvh->GetWidget()->GetView()->Destroy();
+ old_rvh->GetWidget()->SetView(nullptr);
+ }
+ }
+
// Show the new view (or a sad tab) if necessary.
bool new_rfh_has_view = !!render_frame_host_->GetView();
if (!delegate_->IsHidden() && new_rfh_has_view) {
@@ -2181,14 +2228,6 @@ void RenderFrameHostManager::CommitPending() {
render_frame_host_->render_view_host());
}
- // For top-level frames, also hide the old RenderViewHost's view.
- // TODO(creis): As long as show/hide are on RVH, we don't want to hide on
- // subframe navigations or we will interfere with the top-level frame.
- if (is_main_frame &&
- old_render_frame_host->render_view_host()->GetWidget()->GetView()) {
- old_render_frame_host->render_view_host()->GetWidget()->GetView()->Hide();
- }
-
// Make sure the size is up to date. (Fix for bug 1079768.)
delegate_->UpdateRenderViewSizeForRenderManager();
@@ -2210,30 +2249,6 @@ void RenderFrameHostManager::CommitPending() {
delegate_->NotifySwappedFromRenderManager(
old_render_frame_host.get(), render_frame_host_.get(), is_main_frame);
- // The RenderViewHost keeps track of the main RenderFrameHost routing id.
- // If this is committing a main frame navigation, update it and set the
- // routing id in the RenderViewHost associated with the old RenderFrameHost
- // to MSG_ROUTING_NONE.
- if (is_main_frame) {
- RenderViewHostImpl* rvh = render_frame_host_->render_view_host();
- rvh->set_main_frame_routing_id(render_frame_host_->routing_id());
-
- // If the RenderViewHost is transitioning from swapped out to active state,
- // it was reused, so dispatch a RenderViewReady event. For example, this
- // is necessary to hide the sad tab if one is currently displayed. See
- // https://crbug.com/591984.
- //
- // TODO(alexmos): Remove this and move RenderViewReady consumers to use
- // the main frame's RenderFrameCreated instead.
- if (!rvh->is_active())
- rvh->PostRenderViewReady();
-
- rvh->set_is_active(true);
- rvh->set_is_swapped_out(false);
- old_render_frame_host->render_view_host()->set_main_frame_routing_id(
- MSG_ROUTING_NONE);
- }
-
// Swap out the old frame now that the new one is visible.
// This will swap it out and schedule it for deletion when the swap out ack
// arrives (or immediately if the process isn't live).

Powered by Google App Engine
This is Rietveld 408576698