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

content/browser/frame_host/render_frame_host_manager.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_manager.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 1008 matching lines @@
   // SiteInstance to a RenderViewHost (if it is different than our current
   // SiteInstance), so that it is ref counted.  This will happen in
   // CreateRenderView.
   return current_instance->GetRelatedSiteInstance(dest_url);
 }
 
 void RenderFrameHostManager::CreateRenderFrameHostForNewSiteInstance(
     SiteInstance* old_instance,
     SiteInstance* new_instance,
     bool is_main_frame) {
+  int create_render_frame_flags = is_main_frame ? FOR_MAIN_FRAME_NAVIGATION : 0;
   // Ensure that we have created RFHs for the new RFH's opener chain if
   // we are staying in the same BrowsingInstance. This allows the new RFH
   // to send cross-process script calls to its opener(s).
   int opener_route_id = MSG_ROUTING_NONE;
   if (new_instance->IsRelatedSiteInstance(old_instance)) {
     opener_route_id =
         delegate_->CreateOpenerRenderViewsForRenderManager(new_instance);
     if (CommandLine::ForCurrentProcess()->HasSwitch(
             switches::kSitePerProcess)) {
       // Ensure that the frame tree has RenderFrameProxyHosts for the new
       // SiteInstance in all nodes except the current one.
       frame_tree_node_->frame_tree()->CreateProxiesForSiteInstance(
           frame_tree_node_, new_instance);
+      // RenderFrames in different processes from their parent RenderFrames

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

nit: s/different processes/different SiteInstance/

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

nit: s/RenderFrames/RenderFrame(Host)s/

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

Changed RenderWidget(Host)s to just RenderWidgets. Use of RenderFrames is
correct since we are talking about living in different processes.

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

Might processes be better? We use SiteInstance right now to mean that they are
in different processes, but eventually not all SiteInstances will be isolated,
and in that case we need a different way to test whether it is the same process
or not.

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

I think we do want to say SiteInstance here.

If two frames are in the same SiteInstance (even if they're cross-site), they're
definitely in the same process.  And if two frames are in different
SiteInstances, they might be in the same process (e.g., process sharing for
non-protected sites), but we should probably give them separate widgets anyway.

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

I'm not sure I agree. If a non-protected SiteInstance creates an iframe of
another non-protected SiteInstance, the frame will not get a widget. It would
create confusion on the rendering side if it did.

[Charlie Reis, 2014/10/08 21:21:13]

I'm skeptical about that.  It doesn't make sense to me to use the same
RenderWidgetHost for two different SiteInstances, even if they share a process.

That said, I'm also having trouble constructing a scenario where that could
happen, since we shouldn't be putting an iframe in a different process than its
parent unless the two need to be isolated from each other.  I would be ok
asserting that it can't happen, and see if we need to revisit it.

+      // in the frame tree require RenderWidget(Host)s for rendering
+      // and processing input events.
+      if (frame_tree_node_->parent() &&
+          frame_tree_node_->parent()->render_manager()->
+              current_frame_host()->GetSiteInstance() == new_instance)

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

Shouldn't this be "!=" instead of "=="?

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

Yes! That explains a lot about some of the problems I am seeing right now.
Thanks Nasko! :)

+        create_render_frame_flags |= CREATE_RENDER_WIDGET_HOST;
     }
   }
 
+  create_render_frame_flags |= delegate_->IsHidden() ? HIDDEN : 0;
+
   // Create a non-swapped-out RFH with the given opener.
   int route_id = CreateRenderFrame(
-      new_instance, opener_route_id, false, is_main_frame,
-      delegate_->IsHidden());
+      new_instance, opener_route_id, create_render_frame_flags);
   if (route_id == MSG_ROUTING_NONE) {
     pending_render_frame_host_.reset();
     return;
   }
 }
 
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
     SiteInstance* site_instance,
     int view_routing_id,
     int frame_routing_id,
@@ skipping 20 matching lines @@
                                                      render_frame_delegate_,
                                                      frame_tree,
                                                      frame_tree_node_,
                                                      frame_routing_id,
                                                      swapped_out).release());
   return render_frame_host.Pass();
 }
 
 int RenderFrameHostManager::CreateRenderFrame(SiteInstance* instance,
                                               int opener_route_id,
-                                              bool swapped_out,
-                                              bool for_main_frame_navigation,
-                                              bool hidden) {
+                                              int flags) {
+  bool swapped_out = flags & SWAPPED_OUT;
   CHECK(instance);
-  DCHECK(!swapped_out || hidden); // Swapped out views should always be hidden.
+  // Swapped out views should always be hidden.
+  DCHECK(!swapped_out || (flags & HIDDEN));
 
   // TODO(nasko): Remove the following CHECK once cross-site navigation no
   // longer relies on swapped out RFH for the top-level frame.
   if (!frame_tree_node_->IsMainFrame()) {
     CHECK(!swapped_out);
   }
 
   scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
   RenderFrameHostImpl* frame_to_announce = NULL;
   int routing_id = MSG_ROUTING_NONE;
@@ skipping 27 matching lines @@
         int required_bindings = pending_web_ui()->GetBindings();
         RenderViewHost* rvh = new_render_frame_host->render_view_host();
         if ((rvh->GetEnabledBindings() & required_bindings) !=
                 required_bindings) {
           rvh->AllowBindings(required_bindings);
         }
       }
     }
   } else {
     // Create a new RenderFrameHost if we don't find an existing one.
-    new_render_frame_host = CreateRenderFrameHost(
-        instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE, swapped_out, hidden);
+    new_render_frame_host = CreateRenderFrameHost(instance,
+                                                  MSG_ROUTING_NONE,
+                                                  MSG_ROUTING_NONE,
+                                                  swapped_out,
+                                                  flags & HIDDEN);
     RenderViewHostImpl* render_view_host =
         new_render_frame_host->render_view_host();
     int proxy_routing_id = MSG_ROUTING_NONE;
 
     // Prevent the process from exiting while we're trying to navigate in it.
     // Otherwise, if the new RFH is swapped out already, store it.
     if (!swapped_out) {
       new_render_frame_host->GetProcess()->AddPendingView();
     } else {
       proxy = new RenderFrameProxyHost(
           new_render_frame_host->GetSiteInstance(), frame_tree_node_);
       proxy_hosts_[instance->GetId()] = proxy;
       proxy_routing_id = proxy->GetRoutingID();
       if (frame_tree_node_->IsMainFrame())
         proxy->TakeFrameHostOwnership(new_render_frame_host.Pass());
     }
 
     bool success = InitRenderView(render_view_host,
                                   opener_route_id,
                                   proxy_routing_id,
-                                  for_main_frame_navigation);
+                                  flags & FOR_MAIN_FRAME_NAVIGATION);
     if (success) {
       if (frame_tree_node_->IsMainFrame()) {
         // Don't show the main frame's view until we get a DidNavigate from it.
         render_view_host->GetView()->Hide();
       } else if (!swapped_out) {
         // Init the RFH, so a RenderFrame is created in the renderer.
         DCHECK(new_render_frame_host.get());
         success = InitRenderFrame(new_render_frame_host.get());
       }
       if (swapped_out) {
         proxy_hosts_[instance->GetId()]->InitRenderFrameProxy();
       }
     } else if (!swapped_out && pending_render_frame_host_) {
       CancelPending();
     }
     routing_id = render_view_host->GetRoutingID();
     frame_to_announce = new_render_frame_host.get();
   }
 
+  if (flags & CREATE_RENDER_WIDGET_HOST)
+    new_render_frame_host->CreateRenderWidgetHost(render_widget_delegate_,

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

Is there ever a case where we need to create a RenderWidgetHost for a
RenderFrameHost at point later than the creation?
If not, why not create the widget along with the creation of the
RenderFrameHost? This will also allow us to send only one IPC down to the
renderer instead of having this extra one. Also, we avoid possible races due to
having two separate IPCs and events happening in-between.

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

+1.  We do something similar for creating the RenderView and main RenderFrame at
the same time, if I understand correctly.

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

I had thought of this, but the comment in line 1125 talks about reusing a
RenderFrameHost that had previously been attached to a RenderFrameProxyHost.

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

That's only for main frames, which store their RFH inside the
RenderFrameProxyHost.  It'll go away when we remove swappedout://.  (I don't
we're planning to create a separate widget for main frames before then, are we?)

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

Probably not. This is good to know, I have made the change.

+                                                  flags & HIDDEN);
+
   // Use this as our new pending RFH if it isn't swapped out.
   if (!swapped_out)
     pending_render_frame_host_ = new_render_frame_host.Pass();
 
   // If a brand new RFH was created, announce it to observers.
   if (frame_to_announce)
     render_frame_delegate_->RenderFrameCreated(frame_to_announce);
 
   return routing_id;
 }
@@ skipping 524 matching lines @@
 void RenderFrameHostManager::DeleteRenderFrameProxyHost(
     SiteInstance* instance) {
   RenderFrameProxyHostMap::iterator iter = proxy_hosts_.find(instance->GetId());
   if (iter != proxy_hosts_.end()) {
     delete iter->second;
     proxy_hosts_.erase(iter);
   }
 }
 
 }  // namespace content