Move more code behind IsSwappedOutStateForbidden() checks.

content/renderer/render_frame_impl.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/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 490 matching lines @@
   return media::Context3D(provider->ContextGL(), provider->GrContext());
 }
 #endif
 
 bool IsReload(FrameMsg_Navigate_Type::Value navigation_type) {
   return navigation_type == FrameMsg_Navigate_Type::RELOAD ||
          navigation_type == FrameMsg_Navigate_Type::RELOAD_IGNORING_CACHE ||
          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;
 
 }  // namespace
 
 // static
 RenderFrameImpl* RenderFrameImpl::Create(RenderViewImpl* render_view,
                                          int32 routing_id) {
   DCHECK(routing_id != MSG_ROUTING_NONE);
 
@@ skipping 160 matching lines @@
 RenderFrameImpl::~RenderFrameImpl() {
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_, RenderFrameGone());
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_, OnDestruct());
 
 #if defined(VIDEO_HOLE)
   if (contains_media_player_)
     render_view_->UnregisterVideoHoleFrame(this);
 #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.
     // TODO(nasko): Add a check that the |main_render_frame_| of |render_view_|
     // is |this|, once the object is no longer leaked.
     // See https://crbug.com/464764.
     render_view_->main_render_frame_ = nullptr;
   }
 
   render_view_->UnregisterRenderFrame(this);
@@ skipping 397 matching lines @@
 void RenderFrameImpl::OnSwapOut(
     int proxy_routing_id,
     bool is_loading,
     const FrameReplicationState& replicated_frame_state) {
   TRACE_EVENT1("navigation", "RenderFrameImpl::OnSwapOut", "id", routing_id_);
   RenderFrameProxy* proxy = NULL;
   bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
       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
     // a different process.  This involves running the unload handler and
     // clearing the page.  We also allow this process to exit if there are no
     // other active RenderFrames in it.
 
     // Send an UpdateState message before we get swapped out.
     render_view_->SyncNavigationState();
 
@@ skipping 20 matching lines @@
     // willCheckAndDispatchMessageEvent() that needs the proxy.
     if (proxy)
       set_render_frame_proxy(proxy);
 
     // Now that we're swapped out and filtering IPC messages, stop loading to
     // ensure that no other in-progress navigation continues.  We do this here
     // to avoid sending a DidStopLoading message to the browser process.
     // 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,
     // if one is created, that will replace this frame.
     if (!is_main_frame && proxy)
       proxy->web_frame()->initializeFromFrame(frame_);
 
     // Replace the page with a blank dummy URL. The unload handler will not be
     // 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
     // stop compositing.
     // TODO(creis): Support this for subframes as well.
     if (is_main_frame) {
       render_view_->webview()->setVisibilityState(
           blink::WebPageVisibilityStateHidden, false);
     }
   }
 
   // It is now safe to show modal dialogs again.
   // TODO(creis): Deal with modal dialogs from subframes.
   if (is_main_frame)
     render_view_->suppress_dialogs_until_swap_out_ = false;
 
   Send(new FrameHostMsg_SwapOut_ACK(routing_id_));
 
   RenderViewImpl* render_view = render_view_.get();
 
   // 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
   // state passed by the process that is now rendering the frame.
   // TODO(alexmos): We cannot yet do this for swapped-out main frames, because
   // in that case we leave the LocalFrame as the main frame visible to Blink
   // and don't call swap() above. Because swap() is what creates a RemoteFrame
   // 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.
   // TODO(nasko): Remove the dependency on RenderViewImpl here and ref count
   // the process based on the lifetime of this RenderFrameImpl object.
   if (is_main_frame) {
     render_view->WasSwappedOut();
   }
 }
 
@@ skipping 3749 matching lines @@
 #elif defined(ENABLE_BROWSER_CDMS)
         cdm_manager_,
 #endif
         this);
   }
 
   return cdm_factory_;
 }
 
 }  // namespace content