Fix leaking of RenderFrames.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 06a15f8a1ac0929f4dfc7b34f907215a22e32675..12f4addb0dbddd20e783c6c362ba2621cea86aa6 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -239,11 +239,18 @@ RenderFrameHostImpl::~RenderFrameHostImpl() {
   if (delegate_ && render_frame_created_)
     delegate_->RenderFrameDeleted(this);
 
-  // If this was swapped out, it already decremented the active frame count of
-  // the SiteInstance it belongs to.
-  if (IsRFHStateActive(rfh_state_))
+  if (IsRFHStateActive(rfh_state_)) {
+    // If this was swapped out, it already decremented the active frame count of
+    // the SiteInstance it belongs to.
     GetSiteInstance()->decrement_active_frame_count();
 
+    // In the case of swapping with a RenderFrameProxy, cleanup is already
+    // handled on the renderer process side. However, in all other cases the
+    // corresponding RenderFrame also needs to be cleaned up.

[nasko, 2015/10/29 23:55:38]

I'm not correct here. The case of main frame and same site subframe, where the
main frame removes the subframe from the tree. It will result in a detach
message from the RenderFrame, which the renderer will cleanup right after the
IPC is sent.

In that case, we will send an extra IPC that will not have a listener, so it
shouldn't really be a problem, but I still dislike it. I will think of how to
fix this tomorrow, though the strawman is to add DETACHED state into rfh_state_,
which we set in the FrameHostMsg_Detach and handle properly here.

+    if (render_frame_created_)
+      Send(new FrameMsg_Detach(routing_id_));
+  }
+
   // NULL out the swapout timer; in crash dumps this member will be null only if
   // the dtor has run.
   swapout_event_monitor_timeout_.reset();