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

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/logging.h"
@@ skipping 591 matching lines @@
       old_render_frame_host->render_view_host(), frame_tree_node_);
   CHECK(proxy_hosts_.insert(std::make_pair(old_site_instance_id, proxy)).second)
       << "Inserting a duplicate item.";
 
   // Tell the old RenderFrameHost to swap out and be replaced by the proxy.
   old_render_frame_host->SwapOut(proxy, true);
 
   // 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.
     proxy->TakeFrameHostOwnership(old_render_frame_host.Pass());
   }
 }
 
 void RenderFrameHostManager::DiscardUnusedFrame(
     scoped_ptr<RenderFrameHostImpl> render_frame_host) {
   // TODO(carlosk): this code is very similar to what can be found in
   // SwapOutOldFrame and we should see that these are unified at some point.
 
   // If the SiteInstance for the pending RFH is being used by others don't
   // delete the RFH. Just swap it out and it can be reused at a later point.

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

Maybe update the first sentence here as well.

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

Done.

   SiteInstanceImpl* site_instance = render_frame_host->GetSiteInstance();
   if (site_instance->HasSite() && site_instance->active_frame_count() > 1) {
     // Any currently suspended navigations are no longer needed.
     render_frame_host->CancelSuspendedNavigations();
 
     CHECK(!GetRenderFrameProxyHost(site_instance));
     RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
         site_instance, render_frame_host->render_view_host(), frame_tree_node_);
     proxy_hosts_[site_instance->GetId()] = proxy;
 
     // Check if the RenderFrameHost is already swapped out, to avoid swapping it
     // out again.
     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();
   }
 }
 
 void RenderFrameHostManager::MoveToPendingDeleteHosts(
     scoped_ptr<RenderFrameHostImpl> render_frame_host) {
   // |render_frame_host| will be deleted when its SwapOut ACK is received, or
   // when the timer times out, or when the RFHM itself is deleted (whichever
@@ skipping 243 matching lines @@
   RenderFrameProxyHostMap::iterator iter =
       node->render_manager()->proxy_hosts_.find(site_instance_id);
   if (iter != node->render_manager()->proxy_hosts_.end()) {
     RenderFrameProxyHost* proxy = iter->second;
     // Delete the proxy.  If it is for a main frame (and thus the RFH is stored
     // in the proxy) and it was still pending swap out, move the RFH to the
     // pending deletion list first.
     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());
     }
     delete proxy;
     node->render_manager()->proxy_hosts_.erase(site_instance_id);
   }
 
   return true;
 }
@@ skipping 517 matching lines @@
   return true;
 }
 
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     SiteInstance* instance,
     WebUIImpl* web_ui,
     int opener_route_id,
     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);
+  CHECK_IMPLIES(is_site_per_process, !swapped_out);
+
   // Swapped out views should always be hidden.
-  DCHECK(!swapped_out || (flags & CREATE_RF_HIDDEN));
-
-  // 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);
+  DCHECK_IMPLIES(swapped_out, (flags & CREATE_RF_HIDDEN));
 
   scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
   bool success = true;
   if (view_routing_id_ptr)
     *view_routing_id_ptr = MSG_ROUTING_NONE;
 
   // We are creating a pending, speculative or swapped out RFH here. We should
   // never create it in the same SiteInstance as our current RFH.
   CHECK_NE(render_frame_host_->GetSiteInstance(), instance);
 
   // Check if we've already created an RFH for this SiteInstance.  If so, try
   // to re-use the existing one, which has already been initialized.  We'll
   // 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.
     // Prevent the process from exiting while we're trying to use it.
     if (!swapped_out) {
       new_render_frame_host = proxy->PassFrameHostOwnership();
       new_render_frame_host->GetProcess()->AddPendingView();
 
       proxy_hosts_.erase(instance->GetId());
       delete proxy;
@@ skipping 26 matching lines @@
       // Remember that InitRenderView also created the RenderFrameProxy.
       if (swapped_out)
         proxy->set_render_frame_proxy_created(true);
       if (frame_tree_node_->IsMainFrame()) {
         // Don't show the main frame's view until we get a DidNavigate from it.
         // Only the RenderViewHost for the top-level RenderFrameHost has a
         // RenderWidgetHostView; RenderWidgetHosts for out-of-process iframes
         // 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());
       }
     }
 
     if (success) {
       if (view_routing_id_ptr)
         *view_routing_id_ptr = render_view_host->GetRoutingID();
     }
@@ skipping 21 matching lines @@
   }
   return nullptr;
 }
 
 int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
   // A RenderFrameProxyHost should never be created in the same SiteInstance as
   // the current RFH.
   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();
 }
 
 void RenderFrameHostManager::CreateProxiesForChildFrame(FrameTreeNode* child) {
   for (const auto& pair : proxy_hosts_) {
     child->render_manager()->CreateRenderFrameProxy(
         pair.second->GetSiteInstance());
   }
 }
 
@@ skipping 13 matching lines @@
   InitRenderView(render_view_host, opener_route_id, proxy->GetRoutingID(),
                  source->IsMainFrame());
   proxy->set_render_frame_proxy_created(true);
 }
 
 bool RenderFrameHostManager::InitRenderView(
     RenderViewHostImpl* render_view_host,
     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;
 
   // Ensure the renderer process is initialized before creating the
   // RenderView.
   if (!render_view_host->GetProcess()->Init())
     return false;
 
   // If the ongoing navigation is to a WebUI and the RenderView is not in a
@@ skipping 194 matching lines @@
 
   // Swap out the old frame now that the new one is visible.
   // This will swap it out and then put it on the proxy list (if there are other
   // active views in its SiteInstance) or schedule it for deletion when the swap
   // out ack arrives (or immediately if the process isn't live).
   // In the --site-per-process case, old subframe RHFs are not kept alive inside
   // the proxy.
   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 =
         proxy_hosts_.find(render_frame_host_->GetSiteInstance()->GetId());
     if (iter != proxy_hosts_.end()) {
       delete iter->second;
       proxy_hosts_.erase(iter);
     }
 
     // If this is a subframe, it should have a CrossProcessFrameConnector
@@ skipping 281 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