Always swap with a replacement proxy in OnSwapOut.

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 <utility>
 #include <vector>
@@ skipping 1523 matching lines @@
   // 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 deleted.
   if (SiteIsolationPolicy::UseSubframeNavigationEntries())
     SendUpdateState();
   else
     render_view_->SendUpdateState();
 
-  // If we need a proxy to replace this, create it now so its routing id is
-  // registered for receiving IPC messages.
-  if (proxy_routing_id != MSG_ROUTING_NONE) {
-    proxy = RenderFrameProxy::CreateProxyToReplaceFrame(
-        this, proxy_routing_id, replicated_frame_state.scope);
-  }
+  // There should always be a proxy to replace this RenderFrame.  Create it now
+  // so its routing id is registered for receiving IPC messages.
+  CHECK_NE(proxy_routing_id, MSG_ROUTING_NONE);
+  proxy = RenderFrameProxy::CreateProxyToReplaceFrame(
+      this, proxy_routing_id, replicated_frame_state.scope);
 
   // Synchronously run the unload handler before sending the ACK.
   // TODO(creis): Call dispatchUnloadEvent unconditionally here to support
   // unload on subframes as well.
   if (is_main_frame_)
     frame_->dispatchUnloadEvent();
 
   // Swap out and stop sending any IPC messages that are not ACKs.
   if (is_main_frame_)
     render_view_->SetSwappedOut(true);
 
   // Set the proxy here, since OnStop() below could cause an onload event
   // handler to execute, which could trigger code such as
   // willCheckAndDispatchMessageEvent() that needs the proxy.
-  if (proxy)
-    set_render_frame_proxy(proxy);
+  set_render_frame_proxy(proxy);
 
   // 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)
+  if (!is_main_frame_)
     proxy->web_frame()->initializeFromFrame(frame_);
 
   // 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);
   }
 
   Send(new FrameHostMsg_SwapOut_ACK(routing_id_));
 
   RenderViewImpl* render_view = render_view_.get();
   bool is_main_frame = is_main_frame_;
   int routing_id = GetRoutingID();
 
   // Now that all of the cleanup is complete and the browser side is notified,
   // start using the RenderFrameProxy, if one is created.

[Charlie Reis, 2016/04/22 19:11:34]

nit: Drop "if one is created"

[alexmos, 2016/04/25 18:17:08]

Done.

-  if (proxy) {
-    // The swap call deletes this RenderFrame via frameDetached.  Do not access
-    // any members after this call.
-    // TODO(creis): WebFrame::swap() can return false.  Most of those cases
-    // should be due to the frame being detached during unload (in which case
-    // the necessary cleanup has happened anyway), but it might be possible for
-    // it to return false without detaching.  Catch those cases below to track
-    // down https://crbug.com/575245.
-    frame_->swap(proxy->web_frame());
+  //
+  // The swap call deletes this RenderFrame via frameDetached.  Do not access
+  // any members after this call.
+  //
+  // TODO(creis): WebFrame::swap() can return false.  Most of those cases
+  // should be due to the frame being detached during unload (in which case
+  // the necessary cleanup has happened anyway), but it might be possible for
+  // it to return false without detaching.  Catch those cases below to track
+  // down https://crbug.com/575245.
+  frame_->swap(proxy->web_frame());
 
-    // For main frames, the swap should have cleared the RenderView's pointer to
-    // this frame.
-    if (is_main_frame) {
-      base::debug::SetCrashKeyValue("swapout_frame_id",
-                                    base::IntToString(routing_id));
-      base::debug::SetCrashKeyValue("swapout_proxy_id",
-                                    base::IntToString(proxy->routing_id()));
-      base::debug::SetCrashKeyValue(
-          "swapout_view_id", base::IntToString(render_view->GetRoutingID()));
-      CHECK(!render_view->main_render_frame_);
-    }
+  // For main frames, the swap should have cleared the RenderView's pointer to
+  // this frame.
+  if (is_main_frame) {
+    base::debug::SetCrashKeyValue("swapout_frame_id",
+                                  base::IntToString(routing_id));
+    base::debug::SetCrashKeyValue("swapout_proxy_id",
+                                  base::IntToString(proxy->routing_id()));
+    base::debug::SetCrashKeyValue(
+        "swapout_view_id", base::IntToString(render_view->GetRoutingID()));
+    CHECK(!render_view->main_render_frame_);
+  }
 
-    if (is_loading)
-      proxy->OnDidStartLoading();
-  }
+  if (is_loading)
+    proxy->OnDidStartLoading();
 
   // Initialize the WebRemoteFrame with the replication state passed by the
   // process that is now rendering the frame.
-  if (proxy)
-    proxy->SetReplicatedState(replicated_frame_state);
+  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();
 }
 
 void RenderFrameImpl::OnDeleteFrame() {
   // TODO(nasko): If this message is received right after a commit has
@@ skipping 4412 matching lines @@
                                     int match_count,
                                     int ordinal,
                                     const WebRect& selection_rect,
                                     bool final_status_update) {
   Send(new FrameHostMsg_Find_Reply(routing_id_, request_id, match_count,
                                    selection_rect, ordinal,
                                    final_status_update));
 }
 
 }  // namespace content