Fix leaking of RenderFrames.

content/renderer/render_frame_impl.h

Index: content/renderer/render_frame_impl.h
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
index e5146f415d8201e860b63d368a8b865fbc381c49..2599a5574e56dde1427571fd203afb4fbfdb4e1e 100644
--- a/content/renderer/render_frame_impl.h
+++ b/content/renderer/render_frame_impl.h
@@ -167,6 +167,9 @@ class CONTENT_EXPORT RenderFrameImpl
                           CompositorDependencies* compositor_deps,
                           const FrameMsg_NewFrame_WidgetParams& params);
 
+  // Detaches a RenderFrame identified by |routing_id|.
+  static void DetachFrame(int routing_id);
+
   // Returns the RenderFrameImpl for the given routing ID.
   static RenderFrameImpl* FromRoutingID(int routing_id);
 
@@ -883,6 +886,10 @@ class CONTENT_EXPORT RenderFrameImpl
   // ServiceProvider.
   mojo::ServiceProviderPtr ConnectToApplication(const GURL& url);
 
+  void set_is_browser_initiated_detach() {
+    is_browser_initiated_detach_ = true;
+  }
+
   // Stores the WebLocalFrame we are associated with.  This is null from the
   // constructor until SetWebFrame is called, and it is null after
   // frameDetached is called until destruction (which is asynchronous in the
@@ -898,6 +905,11 @@ class CONTENT_EXPORT RenderFrameImpl
   // or it is a main frame.
   bool is_local_root_;
 
+  // When a frame is detached in response to a messge from the browser process,
+  // this RenderFrame should not be sending notifications back to it. This
+  // boolean is used to indicate this case.
+  bool is_browser_initiated_detach_;

[Charlie Reis, 2015/10/30 20:15:44]

nit: in_browser_initiated_detach_, perhaps?

Although, I'm having trouble with this.  On the one hand, it looks and sounds
like we're trying to track whether we've already started detaching this frame,
and thus shouldn't repeat some tasks (like telling the browser process).  That
seems like it could apply to the other leak as well.

On the other hand, it's being used to indicate whether a frame is being
discarded before it's used, and thus isn't in the tree yet.

We should probably pick one interpretation and stick with it for this CL, and
revise it later if needed.

[nasko, 2015/11/02 22:48:52]

It is meant to indicate that it is a browser initiated detaching. If that is the
case, we shouldn't be telling the browser that we are detaching the frame, as it
already initiated this action.
No, I don't think it is used to indicate that. I realized that it is more
generic and we will need this to fix the other leaks too. So in general, if the
browser process told us to detach a frame, we need to know that and avoid
sending IPCs back to it.
It should only be one - browser told us to detach, so we are in the process of
doing so.

+
   base::WeakPtr<RenderViewImpl> render_view_;
   int routing_id_;
   bool is_swapped_out_;