Avoid creating a RenderWidgetHostView for proxies.

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 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/web_contents/web_contents_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
@@ skipping 4153 matching lines @@
                                         source_id);
 }
 
 int WebContentsImpl::CreateSwappedOutRenderView(
     SiteInstance* instance) {
   int render_view_routing_id = MSG_ROUTING_NONE;
   if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
     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;
 }
 
 void WebContentsImpl::OnUserGesture() {
   // Notify observers.
   FOR_EACH_OBSERVER(WebContentsObserver, observers_, DidGetUserGesture());
 
   ResourceDispatcherHostImpl* rdh = ResourceDispatcherHostImpl::Get();
@@ skipping 151 matching lines @@
 scoped_ptr<WebUIImpl> WebContentsImpl::CreateWebUIForRenderManager(
     const GURL& url) {
   return scoped_ptr<WebUIImpl>(static_cast<WebUIImpl*>(CreateWebUI(url)));
 }
 
 NavigationEntry*
     WebContentsImpl::GetLastCommittedNavigationEntryForRenderManager() {
   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);
   }
 
   // Now that the RenderView has been created, we need to tell it its size.
   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 =
       GetMaxPageIDForSiteInstance(render_view_host->GetSiteInstance());
 
   if (!static_cast<RenderViewHostImpl*>(render_view_host)
            ->CreateRenderView(opener_frame_routing_id, proxy_routing_id,
                               max_page_id, replicated_frame_state,
                               created_with_opener_)) {
     return false;
   }
 
   SetHistoryOffsetAndLengthForView(render_view_host,
                                    controller_.GetLastCommittedEntryIndex(),
                                    controller_.GetEntryCount());
 
 #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();
   }
 #endif
 
   return true;
 }
 
 bool WebContentsImpl::CreateRenderFrameForRenderManager(
@@ skipping 31 matching lines @@
   if (!web_contents_android) {
     web_contents_android = new WebContentsAndroid(this);
     SetUserData(kWebContentsAndroidKey, web_contents_android);
   }
   return web_contents_android;
 }
 
 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)
 
 void WebContentsImpl::SetAllowOtherViews(bool allow) {
   view_->SetAllowOtherViews(allow);
 }
 
 bool WebContentsImpl::GetAllowOtherViews() {
   return view_->GetAllowOtherViews();
@@ skipping 157 matching lines @@
   player_map->erase(it);
 }
 
 void WebContentsImpl::SetForceDisableOverscrollContent(bool force_disable) {
   force_disable_overscroll_content_ = force_disable;
   if (view_)
     view_->SetOverscrollControllerEnabled(CanOverscrollContent());
 }
 
 }  // namespace content