Move more code behind IsSwappedOutStateForbidden() checks.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 2113dc9edc6a936d431e7b477241772597d80a1c..b1259bcd1fabce47af83c1dec950172b779b1ade 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -508,6 +508,11 @@ bool IsReload(FrameMsg_Navigate_Type::Value navigation_type) {
          navigation_type == FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL;
 }
 
+bool IsSwappedOutStateForbidden() {

[ncarter (slow), 2015/06/26 20:27:17]

I don't like having IsSwappedOutStateForbidden on both RenderFrameProxy and
RenderFrameImpl. I'll consolidate them in my change.

[nasko, 2015/06/29 08:06:20]

Acknowledged.

+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kSitePerProcess);
+}
+
 RenderFrameImpl::CreateRenderFrameImplFunction g_create_render_frame_impl =
     nullptr;
 
@@ -688,16 +693,18 @@ RenderFrameImpl::~RenderFrameImpl() {
 #endif
 
   if (!is_subframe_) {
-    // When not using --site-per-process, RenderFrameProxy is "owned" by
-    // RenderFrameImpl in the case it is the main frame. Ensure it is deleted
-    // along with this object.
-    if (render_frame_proxy_ &&
-        !base::CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kSitePerProcess)) {
-      // The following method calls back into this object and clears
-      // |render_frame_proxy_|.
-      render_frame_proxy_->frameDetached(
-          blink::WebRemoteFrameClient::DetachType::Remove);
+    if (!IsSwappedOutStateForbidden()) {
+      // When using swapped out frames, RenderFrameProxy is "owned" by
+      // RenderFrameImpl in the case it is the main frame. Ensure it is deleted
+      // along with this object.
+      if (render_frame_proxy_ &&
+          !base::CommandLine::ForCurrentProcess()->HasSwitch(
+              switches::kSitePerProcess)) {

[ncarter (slow), 2015/06/26 20:27:17]

I think you meant to remove this command line check.

[nasko, 2015/06/29 08:06:20]

D'oh!

+        // The following method calls back into this object and clears
+        // |render_frame_proxy_|.
+        render_frame_proxy_->frameDetached(
+            blink::WebRemoteFrameClient::DetachType::Remove);
+      }
     }
 
     // Ensure the RenderView doesn't point to this object, once it is destroyed.
@@ -1115,6 +1122,9 @@ void RenderFrameImpl::OnSwapOut(
       switches::kSitePerProcess);
   bool is_main_frame = !frame_->parent();
 
+  // This codepath should only be hit for subframes when in --site-per-process.
+  CHECK_IMPLIES(!is_main_frame, is_site_per_process);
+
   // Only run unload if we're not swapped out yet, but send the ack either way.
   if (!is_swapped_out_) {
     // Swap this RenderFrame out so the frame can navigate to a page rendered by
@@ -1155,7 +1165,7 @@ void RenderFrameImpl::OnSwapOut(
     // TODO(creis): Should we be stopping all frames here and using
     // StopAltErrorPageFetcher with RenderView::OnStop, or just stopping this
     // frame?
-    if (!is_site_per_process)
+    if (!IsSwappedOutStateForbidden())
       OnStop();
 
     // Transfer settings such as initial drawing parameters to the remote frame,
@@ -1167,7 +1177,7 @@ void RenderFrameImpl::OnSwapOut(
     // run a second time, thanks to a check in FrameLoader::stopLoading.
     // TODO(creis): Need to add a better way to do this that avoids running the
     // beforeunload handler. For now, we just run it a second time silently.
-    if (!is_site_per_process)
+    if (!IsSwappedOutStateForbidden())
       NavigateToSwappedOutURL();
 
     // Let WebKit know that this view is hidden so it can drop resources and
@@ -1190,13 +1200,11 @@ void RenderFrameImpl::OnSwapOut(
 
   // Now that all of the cleanup is complete and the browser side is notified,
   // start using the RenderFrameProxy, if one is created.
-  if (proxy) {
-    if (is_site_per_process || !is_main_frame) {
-      frame_->swap(proxy->web_frame());
+  if (proxy && IsSwappedOutStateForbidden()) {
+    frame_->swap(proxy->web_frame());
 
-      if (is_loading)
-        proxy->OnDidStartLoading();
-    }
+    if (is_loading)
+      proxy->OnDidStartLoading();
   }
 
   // In --site-per-process, initialize the WebRemoteFrame with the replication
@@ -1207,7 +1215,7 @@ void RenderFrameImpl::OnSwapOut(
   // in proxy->web_frame(), the RemoteFrame will not exist for main frames.
   // When we do an unconditional swap for all frames, we can remove
   // !is_main_frame below.
-  if (is_site_per_process && proxy)
+  if (proxy && IsSwappedOutStateForbidden())
     proxy->SetReplicatedState(replicated_frame_state);
 
   // Safe to exit if no one else is using the process.