Remove swapped-out usage in --site-per-process.

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 16e4b588286b0000fa7bf410b3e7e02bc846c7b0..a2366742a13294597b53406c52774b797896436b 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -609,18 +609,16 @@ void RenderFrameHostManager::SwapOutOldFrame(
   // SwapOut creates a RenderFrameProxy, so set the proxy to be initialized.
   proxy->set_render_frame_proxy_created(true);
 
-  bool is_main_frame = frame_tree_node_->IsMainFrame();
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kSitePerProcess) &&
-      !is_main_frame) {
-    // In --site-per-process, subframes delete their RFH rather than storing it
+          switches::kSitePerProcess)) {
+    // In --site-per-process, frames delete their RFH rather than storing it
     // in the proxy.  Schedule it for deletion once the SwapOutACK comes in.
     // TODO(creis): This will be the default when we remove swappedout://.
     MoveToPendingDeleteHosts(old_render_frame_host.Pass());
   } else {
     // We shouldn't get here for subframes, since we only swap subframes when
     // --site-per-process is used.
-    DCHECK(is_main_frame);
+    DCHECK(frame_tree_node_->IsMainFrame());
 
     // The old RenderFrameHost will stay alive inside the proxy so that existing
     // JavaScript window references to it stay valid.
@@ -650,9 +648,14 @@ void RenderFrameHostManager::DiscardUnusedFrame(
     if (!render_frame_host->is_swapped_out())
       render_frame_host->SwapOut(proxy, false);
 
-    if (frame_tree_node_->IsMainFrame())
+    if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSitePerProcess)) {
+      DCHECK(frame_tree_node_->IsMainFrame());
       proxy->TakeFrameHostOwnership(render_frame_host.Pass());
-  } else {
+    }
+  }
+
+  if (render_frame_host) {
     // We won't be coming back, so delete this one.
     ShutdownProxiesIfLastActiveFrameInSiteInstance(render_frame_host.get());
     render_frame_host.reset();
@@ -916,7 +919,9 @@ bool RenderFrameHostManager::ClearProxiesInSiteInstance(
     if (node->IsMainFrame() &&
         proxy->render_frame_host() &&
         proxy->render_frame_host()->rfh_state() ==
-        RenderFrameHostImpl::STATE_PENDING_SWAP_OUT) {
+            RenderFrameHostImpl::STATE_PENDING_SWAP_OUT) {
+      DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSitePerProcess));
       scoped_ptr<RenderFrameHostImpl> swapped_out_rfh =
           proxy->PassFrameHostOwnership();
       node->render_manager()->MoveToPendingDeleteHosts(swapped_out_rfh.Pass());
@@ -1454,14 +1459,14 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     int flags,
     int* view_routing_id_ptr) {
   bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
+  bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kSitePerProcess);
+
   CHECK(instance);
-  // Swapped out views should always be hidden.
-  DCHECK(!swapped_out || (flags & CREATE_RF_HIDDEN));
+  CHECK_IMPLIES(is_site_per_process, !swapped_out);
 
-  // TODO(nasko): Remove the following CHECK once cross-process navigation no
-  // longer relies on swapped out RFH for the top-level frame.
-  if (!frame_tree_node_->IsMainFrame())
-    CHECK(!swapped_out);
+  // Swapped out views should always be hidden.
+  DCHECK_IMPLIES(swapped_out, (flags & CREATE_RF_HIDDEN));
 
   scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
   bool success = true;
@@ -1477,6 +1482,7 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
   // remove it from the list of proxy hosts below if it will be active.
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
   if (proxy && proxy->render_frame_host()) {
+    CHECK(!is_site_per_process);
     if (view_routing_id_ptr)
       *view_routing_id_ptr = proxy->GetRenderViewHost()->GetRoutingID();
     // Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
@@ -1523,7 +1529,8 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
         // will be created later and hidden.
         if (render_view_host->GetView())
           render_view_host->GetView()->Hide();
-      } else if (!swapped_out) {
+      }
+      if (is_site_per_process) {

[Charlie Reis, 2015/06/04 00:02:11]

I don't understand this change.  Can you help explain it?

[nasko, 2015/06/04 14:57:13]

With this CL and --site-per-process, this method is not called anymore for
swapped_out == true, which is guaranteed by the CHECK in the beginning. Since
RenderViewHost for the SiteInstance of the newly created RenderFrameHost might
already exist, due to a subframe for that SiteInstance, it will not do any work
as part of the InitRenderView call. Therefore, we need to create the
RenderFrameImpl for this new RenderFrameHost, otherwise it won't happen. Before
this change, it was only done for subframes, but now it is needed for the main
frame as well. I've put a small comment to clarify this. In general, once we
remove swapped out from regular chrome, this method will be greatly simplified.

         // Init the RFH, so a RenderFrame is created in the renderer.
         DCHECK(new_render_frame_host);
         success = InitRenderFrame(new_render_frame_host.get());
@@ -1565,17 +1572,41 @@ int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
   CHECK(instance);
   CHECK_NE(instance, render_frame_host_->GetSiteInstance());
 
+  bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kSitePerProcess);
+  RenderViewHostImpl* render_view_host = nullptr;
+
+  // Ensure a RenderViewHost exists for |instance|, as it creates the page
+  // level structure in Blink.
+  if (is_site_per_process) {
+    render_view_host =
+        frame_tree_node_->frame_tree()->GetRenderViewHost(instance);
+    if (!render_view_host) {
+      CHECK(frame_tree_node_->IsMainFrame());
+      render_view_host = frame_tree_node_->frame_tree()->CreateRenderViewHost(
+          instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE, true, true);
+    }
+  }
+
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
-  if (proxy && proxy->is_render_frame_proxy_live())
+  if (proxy && proxy->is_render_frame_proxy_live()) {

[Charlie Reis, 2015/06/04 00:02:11]

nit: No braces needed.

[nasko, 2015/06/04 14:57:13]

Done.

     return proxy->GetRoutingID();
+  }
 
   if (!proxy) {
     proxy = new RenderFrameProxyHost(
-        instance, frame_tree_node_->frame_tree()->GetRenderViewHost(instance),
-        frame_tree_node_);
+        instance, render_view_host, frame_tree_node_);

[Charlie Reis, 2015/06/04 00:02:11]

This looks like a regression for non-site-per-process.  Before we passed a RVH
if it existed, and now we definitely pass null.

[nasko, 2015/06/04 14:57:13]

This method will never be called for the main frame without --site-per-process.
We will always go through CreateRenderFrame with swapped_out == true.

     proxy_hosts_[instance->GetId()] = proxy;
   }
-  proxy->InitRenderFrameProxy();
+
+  if (is_site_per_process && frame_tree_node_->IsMainFrame()) {
+    InitRenderView(

[Charlie Reis, 2015/06/04 00:02:11]

Why would we need to call this if the RVH already existed on line 1582?

[nasko, 2015/06/04 14:57:13]

We generally don't need to. But we don't know whether we created it or not. The
method will not do anything if the RVH is already live, so it is safe to call
unconditionally. In general, if it isn't, we definitely need to call it and I've
opted to creating the RV and the RFP in the same IPC, just like we do for the
main frame.

+        render_view_host, MSG_ROUTING_NONE, proxy->GetRoutingID(), true);
+    proxy->set_render_frame_proxy_created(true);
+  } else {
+    proxy->InitRenderFrameProxy();

[Charlie Reis, 2015/06/04 00:02:11]

Why don't we need to call this for main frames in --site-per-process?

[nasko, 2015/06/04 14:57:13]

See the comment above. The IPC to create the RenderView will also create the
proxy. RenderView needs to always be created with the top-level element, be it a
frame or a proxy.

+  }
+
   return proxy->GetRoutingID();
 }
 
@@ -1609,6 +1640,9 @@ bool RenderFrameHostManager::InitRenderView(
     int opener_route_id,
     int proxy_routing_id,
     bool for_main_frame_navigation) {
+  if (!render_view_host->GetProcess()->Init())

[Charlie Reis, 2015/06/04 00:02:11]

The same thing exists on line 1652.  Why do we need it twice?

[nasko, 2015/06/04 14:57:13]

Mistake on my part, just needed to move up.

+    return false;
+
   // We may have initialized this RenderViewHost for another RenderFrameHost.
   if (render_view_host->IsRenderViewLive())
     return true;
@@ -1823,8 +1857,7 @@ void RenderFrameHostManager::CommitPending() {
   SwapOutOldFrame(old_render_frame_host.Pass());
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kSitePerProcess) &&
-      !is_main_frame) {
+          switches::kSitePerProcess)) {
     // Since the new RenderFrameHost is now committed, there must be no proxies
     // for its SiteInstance. Delete any existing ones.
     RenderFrameProxyHostMap::iterator iter =