Make RenderFrame(Host) own a RenderWidget(Host).

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 09d1f1198bc983d993fdb14b44645f0cfb03ec1f..205bd355b66b1ccd80e7d8302269b0f23c3e4b8c 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -713,7 +713,34 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
   frame_tree_node()->navigator()->DidNavigate(this, validated_params);
 }
 
+void RenderFrameHostImpl::CreateRenderWidgetHost(
+    RenderWidgetHostDelegate* delegate,
+    bool hidden) {
+  DCHECK(!render_widget_host_impl_);
+  // TODO(kenrb): After RenderWidgetHost is divorced from RenderViewHost it
+  // will no longer need its own routing_id, and instead can route messages

[Charlie Reis, 2014/10/01 17:01:08]

Is there a benefit to this?  There are still some RenderWidgets that need their
own routing ID (e.g., context menus), so this means some RenderWidgets will have
their own and some won't.  It's probably also nice to avoid having the
RenderWidget IPCs go through RenderFrame's OnMessageReceived.

[kenrb, 2014/10/01 19:27:51]

That's a good point about still needing them for popups. I'll remove the
comment, we can decide to do this later or not.

+  // through its owner's IPC channel.
+  int render_widget_host_impl_routing_id = GetProcess()->GetNextRoutingID();
+  render_widget_host_impl_.reset(
+    new RenderWidgetHostImpl(delegate,

[nasko, 2014/10/01 16:37:14]

nit: needs two more spaces of indentation.

[kenrb, 2014/10/01 19:27:51]

Done.

+                             GetProcess(),
+                             render_widget_host_impl_routing_id,
+                             hidden));
+
+  new RenderWidgetHostViewChildFrame(render_widget_host_impl_.get());

[nasko, 2014/10/01 16:37:14]

Who will own this new child frame object?

[kenrb, 2014/10/01 19:27:51]

The RenderWidgetHostImpl that is passed in the argument ends up owning it.

[Charlie Reis, 2014/10/02 00:06:57]

Please add a comment for that, since it's not obvious from this code.

[kenrb, 2014/10/02 20:20:47]

Done. The comment I added is a bit more accurate than that.

+
+  Send(new FrameMsg_NewWidgetForFrame(routing_id_,
+                                      render_widget_host_impl_routing_id,
+                                      hidden));
+}
+
 RenderWidgetHostImpl* RenderFrameHostImpl::GetRenderWidgetHost() {
+  if (render_widget_host_impl_)
+    return render_widget_host_impl_.get();
+
+  // TODO(kenrb): When RenderViewHost no longer inherits RenderWidgetHost,

[nasko, 2014/10/01 16:37:14]

When that happens, wouldn't we want to go up the chain of frames and get the
right widget? Or do you plan on passing it into each RFH when created?

[Charlie Reis, 2014/10/01 17:01:08]

We'll need to walk up the tree, since render_widget_host_impl_ is a scoped_ptr. 
(Each frame in the group can't own it.)

[kenrb, 2014/10/01 19:27:51]

I think we should only try to access a widget on RenderFrameHosts that already
actually them, but we will see when we get there.

[Charlie Reis, 2014/10/02 00:06:57]

That makes this a bit of a strange method.  How would new callers know whether
they're allowed to call it or not?

[kenrb, 2014/10/02 20:20:47]

My view of this is that anybody who has business talking to a RenderWidgetHost
should already know where they are, so returning NULL for a RFH that doesn't
have a RWH gives a good signal that something isn't right. Currently the only
caller of this method calls it on the main frame.

[nasko, 2014/10/02 20:41:43]

Wouldn't that mean that each of those places that has business talking to
RenderWidgetHost (and knowing what it is) will have to include code to walk the
tree up to find where to get it from?

[kenrb, 2014/10/03 17:23:32]

RenderWidgetHost represents the content to the platform. Given its function, I
don't think there should be cases where you have a pointer to an arbitrary
RenderFrameHost and you need to find out what widget its rendering is happening
through.

+  // we can remove this fallback. Currently it is only used for the main
+  // frame.
   return static_cast<RenderWidgetHostImpl*>(render_view_host_);
 }