Start using RenderFrameProxyHost objects.

content/browser/frame_host/frame_tree.h

Index: content/browser/frame_host/frame_tree.h
diff --git a/content/browser/frame_host/frame_tree.h b/content/browser/frame_host/frame_tree.h
index 67e61a98195e27964629342a09c5044d68cb2b45..8087b0f38daad07f719dfdb1bd78ae99831b74e2 100644
--- a/content/browser/frame_host/frame_tree.h
+++ b/content/browser/frame_host/frame_tree.h
@@ -71,6 +71,12 @@ class CONTENT_EXPORT FrameTree {
                                 const std::string& frame_name);
   void RemoveFrame(FrameTreeNode* child);
 
+  // Walks the entire frame tree and creates a RenderFrameProxyHost for the
+  // given |site_instance| in every node except the |source| one.

[Charlie Reis, 2014/07/24 22:36:29]

This is pretty ambiguous as written, so let's expand it a bit.  What happens if
there's already an active RenderFrameHost or an existing RenderFrameProxyHost
for the SiteInstance in a given frame?  Presumably it leaves the frame alone,
rather than creating a RFPH?

Maybe it would help to say what precondition we're expecting (e.g., the
SiteInstance isn't present in any of the frames of the tree so far?).

[kenrb, 2014/07/25 23:42:05]

I have tried to clarify this. Please review the new comment.

+  void CreateProxiesForSiteInstance(
+      FrameTreeNode* source,
+      const scoped_refptr<SiteInstance>& site_instance);

[Charlie Reis, 2014/07/24 22:36:29]

Why pass the scoped_refptr& here when we're just passing it as a SiteInstance*
in the body?  Seems like the other methods on this class also use SiteInstance*,
so I'm wondering whether there's a need for the scoped_refptr in this case.

[nasko, 2014/07/25 07:13:20]

See my response in RFHM.

[kenrb, 2014/07/25 23:42:04]

I have changed it to a SiteInstance*, but it needs to be scoped_refptr because
base::Bind requires it for any refcounted types. I am making it create one
within the method, though, to simplify the interface.

[Charlie Reis, 2014/07/28 19:24:28]

Oh, there was a reason.  Darn.

This approach should work, but let's mention that it creates a scoped_refptr for
|site_instance| in the declaration comment, since that can affect behavior
(e.g., deleting it if there are no other references to it when you call this
method).

[ncarter (slow), 2014/07/28 22:30:49]

Wouldn't base::Unretained work if you want it to be a raw ptr?

+
   // Clears process specific-state after a main frame process swap.
   // This destroys most of the frame tree but retains the root node so that
   // navigation state may be kept on it between process swaps. Used to
@@ -100,20 +106,19 @@ class CONTENT_EXPORT FrameTree {
   void SetFrameRemoveListener(
       const base::Callback<void(RenderFrameHost*)>& on_frame_removed);
 
-  // Creates a RenderViewHost for a new main frame RenderFrameHost in the given
+  // Creates a RenderViewHost for a new RenderFrameHost in the given
   // |site_instance|.  The RenderViewHost will have its Shutdown method called
   // when all of the RenderFrameHosts using it are deleted.
-  RenderViewHostImpl* CreateRenderViewHostForMainFrame(
-      SiteInstance* site_instance,
-      int routing_id,
-      int main_frame_routing_id,
-      bool swapped_out,
-      bool hidden);
-
-  // Returns the existing RenderViewHost for a new subframe RenderFrameHost.
+  RenderViewHostImpl* CreateRenderViewHost(SiteInstance* site_instance,
+                                           int routing_id,
+                                           int main_frame_routing_id,
+                                           bool swapped_out,
+                                           bool hidden);
+
+  // Returns the existing RenderViewHost for a new RenderFrameHost.
   // There should always be such a RenderViewHost, because the main frame
   // RenderFrameHost for each SiteInstance should be created before subframes.
-  RenderViewHostImpl* GetRenderViewHostForSubFrame(SiteInstance* site_instance);
+  RenderViewHostImpl* GetRenderViewHost(SiteInstance* site_instance);
 
   // Keeps track of which RenderFrameHosts are using each RenderViewHost.  When
   // the number drops to zero, we call Shutdown on the RenderViewHost.