Note: this is a follow-up to a memory leak discovered and discussed in  https://crbug.com/718489#c30  - #c38.


RenderFrameProxyHost can be constructed with a null |render_view_host| argument.  RenderFrameProxyHost's constructor used to have a CHECK that |render_view_host| argument is never null, but this has changed in r336565 (e.g. see https://codereview.chromium.org/972313002/patch/1060001/1070020).  An example of a callstack that constructs RenderFrameProxyHost with a null |render_view_host| argument:

  1 content::RenderFrameProxyHost::RenderFrameProxyHost(...) ../../content/browser/frame_host/render_frame_proxy_host.cc:68:35
  2 content::RenderFrameHostManager::CreateRenderFrameProxyHost(...) ../../content/browser/frame_host/render_frame_host_manager.cc:817:11
  3 content::RenderFrameHostManager::CreateOuterDelegateProxy(...) ../../content/browser/frame_host/render_frame_host_manager.cc:1744:7
  4 content::WebContentsImpl::AttachToOuterWebContentsFrame(...) ../../content/browser/web_contents/web_contents_impl.cc:1628:19
  5 guest_view::GuestViewMessageFilter::OnAttachToEmbedderFrame(...) ../../components/guest_view/browser/guest_view_message_filter.cc:174:23

When 2 GuestViews have the same storage partition, they are able to look each other up via window.open (see also  issue 794079 ).  Looking another frame via window.open needs to set the frame's opener (to the frame initiating the lookup).  This can lead to the following callstack:

  1 RenderProcessHostImpl::AddRoute(...) ../../content/browser/renderer_host/render_process_host_impl.cc:2207:33
  2 RenderWidgetHostImpl::RenderWidgetHostImpl(...) ../../content/browser/renderer_host/render_widget_host_impl.cc:371:13
  3 RenderWidgetHostFactory::Create(...) ../../content/browser/renderer_host/render_widget_host_factory.cc:25:14
  4 RenderViewHostFactory::Create(...) ../../content/browser/renderer_host/render_view_host_factory.cc:53:24
  5 FrameTree::CreateRenderViewHost(...) ../../content/browser/frame_host/frame_tree.cc:360:40
  6 RenderFrameHostManager::CreateRenderFrameProxy(...) ../../content/browser/frame_host/render_frame_host_manager.cc:1670:56
  7 FrameTree::CreateProxiesForSiteInstance(...) ../../content/browser/frame_host/frame_tree.cc:253:33
  8 RenderFrameHostManager::CreateOpenerProxies(...) ../../content/browser/frame_host/render_frame_host_manager.cc:2226:0
  9 RenderFrameProxyHost::UpdateOpener() ../../content/browser/frame_host/render_frame_proxy_host.cc:216:51
 10 RenderFrameHostManager::DidChangeOpener(...) ../../content/browser/frame_host/render_frame_host_manager.cc:299:18

The problem is that FrameHostManager::CreateRenderFrameProxy will create and leak a RenderViewHost:

  1666  int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
  ...
  1672    RenderViewHostImpl* render_view_host = nullptr;
  1673  
  1674    // Ensure a RenderViewHost exists for |instance|, as it creates the page
  1675    // level structure in Blink.
  1676    render_view_host =
  1677        frame_tree_node_->frame_tree()->GetRenderViewHost(instance);
  1678    if (!render_view_host) {
  1679      CHECK(frame_tree_node_->IsMainFrame());
  1680      render_view_host = frame_tree_node_->frame_tree()->CreateRenderViewHost(
  1681          instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE, true, true);
  1682    }
  1683  
  1684    RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
  1685    if (proxy && proxy->is_render_frame_proxy_live())
  1686      return proxy->GetRoutingID();
  ...

If a |proxy| for |instance| already exists, but doesn't have an associated |render_view_host|, then on line 1678 we will realize that there is no |render_view_host| for |instance| (which is okay), but then we will incorrectly create a |render_view_host| on line 1680 - this RenderViewHostImpl will be leaked.  Normally a newly created RenderViewHostImpl is associated either with RenderFrameHostImpl or RenderFrameProxyHost - constructors of RFHI and RFPH call FrameTree::AddRenderViewHostRef and their destructors FrameTree::AddRenderViewHostRef call FrameTree::ReleaseRenderViewHostRef (which eventually will destroy the RenderViewHostImpl).  In this case, the newly created RenderViewHostImpl is forgotten, because lines 1684-1686 return an already existing, perfectly good RenderFrameProxyHost (which was legitimately created earlier *without* an associated RenderViewHostImpl).

Leaking of RenderViewHostImpl consequently leads to leaking of RenderProcessHostImpl (and leaking of a renderer process).