Avoid creating a RenderWidgetHostView for proxies.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index f7505af877b67c2644609a9ff6920d39dfba84a0..a312c40d170078d65d58453af772f07fc03a8e01 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -4171,9 +4171,7 @@ int WebContentsImpl::CreateSwappedOutRenderView(
     GetRenderManager()->CreateRenderFrameProxy(instance);
   } else {
     GetRenderManager()->CreateRenderFrame(
-        instance, nullptr,
-        CREATE_RF_SWAPPED_OUT | CREATE_RF_FOR_MAIN_FRAME_NAVIGATION |
-            CREATE_RF_HIDDEN,
+        instance, nullptr, CREATE_RF_SWAPPED_OUT | CREATE_RF_HIDDEN,
         &render_view_routing_id);
   }
   return render_view_routing_id;
@@ -4345,28 +4343,18 @@ NavigationEntry*
   return controller_.GetLastCommittedEntry();
 }
 
-bool WebContentsImpl::CreateRenderViewForRenderManager(
+bool WebContentsImpl::CreateRenderWidgetHostViewForRenderManager(
     RenderViewHost* render_view_host,
-    int opener_frame_routing_id,
-    int proxy_routing_id,
-    const FrameReplicationState& replicated_frame_state,
-    bool for_main_frame_navigation) {
-  TRACE_EVENT0("browser,navigation",
-               "WebContentsImpl::CreateRenderViewForRenderManager");
-  // Can be NULL during tests.
-  RenderWidgetHostViewBase* rwh_view;
-  // TODO(kenrb): RenderWidgetHostViewChildFrame special casing is temporary
-  // until RenderWidgetHost is attached to RenderFrameHost. We need to special
-  // case this because RWH is still a base class of RenderViewHost, and child
-  // frame RWHVs are unique in that they do not have their own WebContents.
+    bool main_frame_is_proxy) {

[Charlie Reis, 2015/08/31 19:02:51]

Hmm, I'm not happy with this.  The name leads to some confusion at the call
sites (as I noted earlier), since it's always false for subframes even if the
main frame is a proxy.  Seems like we're trying to pass for_active_main_frame or
something like that?  That's the one case it will be true.

But on the flip side, the main frame proxy case seems like it makes this method
a no-op.  For a method named CreateRWHV, maybe it would be clearer if the
callers checked that in advance when deciding whether to call it?  Or does the
guest case make that too tricky?

[lfg, 2015/09/02 00:32:17]

Done.

+  RenderWidgetHostViewBase* rwh_view = nullptr;
   bool is_guest_in_site_per_process =
       !!browser_plugin_guest_.get() &&
       BrowserPluginGuestMode::UseCrossProcessFramesForGuests();
-  if (!for_main_frame_navigation || is_guest_in_site_per_process) {
+  if (is_guest_in_site_per_process) {
     RenderWidgetHostViewChildFrame* rwh_view_child =
         new RenderWidgetHostViewChildFrame(render_view_host);
     rwh_view = rwh_view_child;
-  } else {
+  } else if (!main_frame_is_proxy) {
     rwh_view = view_->CreateViewForWidget(render_view_host, false);
   }
 
@@ -4374,6 +4362,20 @@ bool WebContentsImpl::CreateRenderViewForRenderManager(
   if (rwh_view)
     rwh_view->SetSize(GetSizeForNewRenderView());
 
+  return !!rwh_view;

[Charlie Reis, 2015/08/31 19:02:51]

No one seems to check the return value.  Seems like it should be a void method.

[lfg, 2015/09/02 00:32:17]

Done.

+}
+
+bool WebContentsImpl::CreateRenderViewForRenderManager(
+    RenderViewHost* render_view_host,
+    int opener_frame_routing_id,
+    int proxy_routing_id,
+    const FrameReplicationState& replicated_frame_state) {
+  TRACE_EVENT0("browser,navigation",
+               "WebContentsImpl::CreateRenderViewForRenderManager");
+
+  CreateRenderWidgetHostViewForRenderManager(
+      render_view_host, proxy_routing_id != MSG_ROUTING_NONE);

[Charlie Reis, 2015/08/31 19:02:51]

What if this were for a subframe?  We would be passing in true for a subframe
proxy_routing_id, not main_frame_is_proxy.

[lfg, 2015/09/02 00:32:17]

This can't be a subframe and have a proxy_routing_id. The subframes RenderView
get initialized in RenderFrameHostManager::CreateRenderFrameProxy (in
site-per-process mode).

Now, if this was a subframe, which could happen if the process crashed between
the initialization in CreateRenderFrameProxy and calling InitRenderView again.
For the purposes of this initialization it doesn't matter, because we don't get
the RenderProcessGone between the two calls to InitRenderView.

I agree this is a bit ugly and not ideal, but I'm not sure on how to improve
this for subframes. Ideally we shouldn't be calling InitRenderView() twice.

Another option is to pass IsMainFrame() into this function and always skip the
RWHV for subframes. WDYT?

+
   // Make sure we use the correct starting page_id in the new RenderView.
   UpdateMaxPageIDIfNecessary(render_view_host);
   int32 max_page_id =
@@ -4393,6 +4395,7 @@ bool WebContentsImpl::CreateRenderViewForRenderManager(
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   // Force a ViewMsg_Resize to be sent, needed to make plugins show up on
   // linux. See crbug.com/83941.
+  RenderWidgetHostView* rwh_view = render_view_host->GetView();
   if (rwh_view) {
     if (RenderWidgetHost* render_widget_host = rwh_view->GetRenderWidgetHost())
       render_widget_host->WasResized();
@@ -4444,7 +4447,7 @@ WebContentsAndroid* WebContentsImpl::GetWebContentsAndroid() {
 bool WebContentsImpl::CreateRenderViewForInitialEmptyDocument() {
   return CreateRenderViewForRenderManager(
       GetRenderViewHost(), MSG_ROUTING_NONE, MSG_ROUTING_NONE,
-      frame_tree_.root()->current_replication_state(), true);
+      frame_tree_.root()->current_replication_state());
 }
 
 #elif defined(OS_MACOSX)