Destroy the old RenderWidgetHostView when swapping out a main frame.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 367951acc517b810c354a23f9e2958c869927969..d3cc7353eeed960b309de0d7743851fbe4b0b2c1 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -4858,14 +4858,13 @@ bool WebContentsImpl::CreateRenderViewForRenderManager(
   TRACE_EVENT0("browser,navigation",
                "WebContentsImpl::CreateRenderViewForRenderManager");
 
-  if (proxy_routing_id == MSG_ROUTING_NONE)
+  RenderViewHostImpl* rvhi = static_cast<RenderViewHostImpl*>(render_view_host);
+
+  if (!rvhi->is_swapped_out())

[lfg, 2016/11/15 04:37:34]

This is also a no-op, but it makes it easier to understand and associate the
creation of the RenderWidgetHostView with a non-swapped-out RenderViewHost.

[Charlie Reis, 2016/11/15 19:59:05]

When you say no-op, you mean is_swapped_out() and proxy_routing_id ==
MSG_ROUTING_NONE are equivalent?  Can you add a DCHECK for that if so?  I wonder
if it would also help to say something like:
// Only create a RenderWidgetHostView if the RVH's main frame is active.

And looking closer, it seems unfortunate that we're adding a call to
RVHI::is_swapped_out().  I was hoping to remove that member entirely, as the
TODO in render_view_host_impl.h indicates.  Basically, it's confusing to try to
track swapped out state on RVH.  Do you think there's a way around this, or do
we need to stick with this concept for now?

[alexmos, 2016/11/17 17:55:57]

See my earlier comment about moving the GetView() destruction to CommitPending,
which might allow using rvhi->is_active() here instead.

Also, let's add a comment here explaining why this is only done for active RVHs
and mention that for reused swapped-out RVHs we do this in CommitPending when
they become active.

[lfg, 2016/11/23 00:27:05]

Added the DCHECK and switched to is_active.

     CreateRenderWidgetHostViewForRenderManager(render_view_host);
 
-  if (!static_cast<RenderViewHostImpl*>(render_view_host)
-           ->CreateRenderView(opener_frame_routing_id,
-                              proxy_routing_id,
-                              replicated_frame_state,
-                              created_with_opener_)) {
+  if (!rvhi->CreateRenderView(opener_frame_routing_id, proxy_routing_id,
+                              replicated_frame_state, created_with_opener_)) {
     return false;
   }