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 18a88eb441030f40bbeb1fb55ee33ec8085b8d4f..5b83b2649eadfa82fd382ddb640d3f9571cdcdec 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -258,11 +258,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.
+    if (!frame_tree_node_->IsMainFrame() && render_frame_created_)

[Charlie Reis, 2015/12/11 20:10:14]

Hmm, this condition and the comment above seem to be talking about different
things.  It's not obvious how we're skipping the IPC in the "swapping with a
RenderFrameProxy" case-- maybe that makes IsRFHStateActive return false?

I think both of these blocks might be easier to read if we split them up. 
Something like:

bool is_active = IsRFHStateActive(rfh_state_);

// If this is swapping out, it already decremented the active frame count
// of the SiteInstance it belongs to.
if (is_active)
  GetSiteInstance()->decrement_active_frame_count();

// If this is swapping with a RenderFrameProxy, the RenderFrame will already
// be deleted in the renderer process. Main frame RenderFrames will be
// cleaned up as part of deleting their RenderView. In all other cases, the
// RenderFrame should be cleaned up (if it exists).
if (is_active && !frame_tree_node_->IsMainFrame() && render_frame_created_)
  Send(new FrameMsg_Detach(routing_id_));

[nasko, 2015/12/14 21:00:02]

Done.

+      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();