Destroy the old RenderWidgetHostView when swapping out a main frame.

content/browser/frame_host/render_frame_host_manager.cc

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 380f6871749da153635007161aed49dc9fd1a3d5..02cfd60c8543999930cd2f6dd3689ee68daccd60 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();
+
+    rvh->set_is_active(false);

[lfg, 2016/11/29 17:41:22]

I had to change this back from the DCHECK, since the DCHECK isn't always true.
The RenderViewHost can be active when the OnSwappedOut is called from the
swapout_event_monitor_timeout_.

[Charlie Reis, 2016/11/29 19:35:55]

Ok.  Please mention that in a comment.

[alexmos, 2016/11/29 19:47:37]

I'm a bit confused by this actually.  We start the swapout timer in
CommitPending -> SwapOutOldFrame -> SwapOut, and that call in CommitPending
happens after we've done old_rvh->set_is_active(false).  How can we still get
here from OnSwappedOut with is_active being true?
Is this because some tests call OnSwappedOut directly, without going through the
timer and CommitPending?

+    rvh->set_is_swapped_out(true);
+  }
+
   for (RFHPendingDeleteList::iterator iter = pending_delete_hosts_.begin();
        iter != pending_delete_hosts_.end();
        iter++) {
@@ -1758,11 +1772,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());
   }
@@ -2163,6 +2172,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) {
@@ -2180,14 +2227,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();
 
@@ -2209,30 +2248,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).