Destroy the old RenderWidgetHostView when swapping out a main frame.

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 4903 matching lines @@
 }
 
 bool WebContentsImpl::CreateRenderViewForRenderManager(
     RenderViewHost* render_view_host,
     int opener_frame_routing_id,
     int proxy_routing_id,
     const FrameReplicationState& replicated_frame_state) {
   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;
   }
 
   SetHistoryOffsetAndLengthForView(render_view_host,
                                    controller_.GetLastCommittedEntryIndex(),
                                    controller_.GetEntryCount());
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   // Force a ViewMsg_Resize to be sent, needed to make plugins show up on
   // linux. See crbug.com/83941.
@@ skipping 379 matching lines @@
   GetMainFrame()->AddMessageToConsole(
       content::CONSOLE_MESSAGE_LEVEL_WARNING,
       base::StringPrintf("This site does not have a valid SSL "
                          "certificate! Without SSL, your site's and "
                          "visitors' data is vulnerable to theft and "
                          "tampering. Get a valid SSL certificate before"
                          " releasing your website to the public."));
 }
 
 }  // namespace content