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..57857df5a2e103322ecb1f10d9739dafe0690a02 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -258,11 +258,20 @@ 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_))
+  bool is_active = IsRFHStateActive(rfh_state_);
+
+  // If this RenderFrameHost is swapped out, it already decremented the active
+  // frame count of the SiteInstance it belongs to.
+  if (is_active)
     GetSiteInstance()->decrement_active_frame_count();
 
+  // If this RenderFrameHost is swapping with a RenderFrameProxyHost, the
+  // RenderFrame will already be deleted in the renderer process. Main frame
+  // RenderFrames will be cleaned up as part of deleting its RenderView. In all
+  // other cases, the RenderFrame should be cleaned up (if it exists).
+  if (is_active && !frame_tree_node_->IsMainFrame() && render_frame_created_)

[dcheng, 2015/12/14 22:14:41]

Nit: I would personally find this easier to read if this were merged into the
above if (is_active)

[Charlie Reis, 2015/12/14 22:21:14]

I asked for this change because the previous comments were hard to follow when
they were merged under the same block.  (The comments were confusingly referring
to the outer condition when placed above an inner condition.)  It's possible to
rephrase the whole thing and merge them into one block, but I think it's fairly
readable as is.

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