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 4a1d2fb83c227dde49a3513ebb6c714b2a3067d5..bd1a6461f5493368859ba6884700f22a94151d73 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -4921,14 +4921,17 @@ bool WebContentsImpl::CreateRenderViewForRenderManager(
   TRACE_EVENT0("browser,navigation",
                "WebContentsImpl::CreateRenderViewForRenderManager");
 
-  if (proxy_routing_id == MSG_ROUTING_NONE)
+  RenderViewHostImpl* rvhi = static_cast<RenderViewHostImpl*>(render_view_host);
+
+  // We only create a RWHV for active RenderViewHosts. When an inactive RVH is
+  // reused, the RWHV is created in RenderFrameHostManager::CommitPending.

[piman, 2017/03/28 20:57:55]

Belated drive-by...

This seems to be assuming it's ok to destroy and then recreate a RWHV on the
same RWHI. I don't think it's true at all - some state is distributed between
RW, RWHI and RWHV and so RWHI must be 1:1 with RWHV, at least without a fair
amount of refactoring. Among others, the RWHV is holding on to and manages
graphics resources sent by the RW, and it leads to a lot of confusion (and
possibly leaks) if the RW ends up talking to different RWHV without knowing
anything about it.


Why are RVH "reused"? Could we not destroy the RWHV if they are?

+  if (rvhi->is_active()) {
+    DCHECK_EQ(MSG_ROUTING_NONE, proxy_routing_id);
     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;
   }