Fix leaking of RenderFrames.

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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/frame_host/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 240 matching lines @@
   // RenderFrameHost during cleanup.
   ClearAllWebUI();
 
   GetProcess()->RemoveRoute(routing_id_);
   g_routing_id_frame_map.Get().erase(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_));
 
   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();
 
   for (const auto& iter: visual_state_callbacks_) {
     iter.second.Run(false);
   }
 
   if (render_widget_host_ &&
       render_widget_host_->owned_by_render_frame_host()) {
@@ skipping 2224 matching lines @@
   *dst = src;
 
   if (src.routing_id != -1)
     dst->tree_id = RoutingIDToAXTreeID(src.routing_id);
 
   if (src.parent_routing_id != -1)
     dst->parent_tree_id = RoutingIDToAXTreeID(src.parent_routing_id);
 }
 
 }  // namespace content