Fix RenderFrameHost lifetime and clean up CommitPending.

content/browser/frame_host/render_frame_host_impl.h

Index: content/browser/frame_host/render_frame_host_impl.h
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
index a59b5c9770a0885cac7335cdd9e80b45d25f31cf..bb4032cc5a91872d0973785fbb18267cd3267166 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -75,17 +75,13 @@ class CONTENT_EXPORT RenderFrameHostImpl
     // RenderFrame.
     STATE_DEFAULT = 0,
     // The RFH has not received the SwapOutACK yet, but the new page has
-    // committed in a different RFH. The number of active frames of the RFH
-    // SiteInstanceImpl is not zero. Upon reception of the SwapOutACK, the RFH
-    // will be swapped out.
+    // committed in a different RFH.  Upon reception of the SwapOutACK, the RFH
+    // will either enter STATE_SWAPPED_OUT (if it is a main frame and there are
+    // other active frames in its SiteInstance) or it will be deleted.
     STATE_PENDING_SWAP_OUT,
-    // The RFH has not received the SwapOutACK yet, but the new page has
-    // committed in a different RFH. The number of active frames of the RFH
-    // SiteInstanceImpl is zero. Upon reception of the SwapOutACK, the RFH will
-    // be shutdown.
-    STATE_PENDING_SHUTDOWN,

[Charlie Reis, 2014/10/01 22:43:14]

We didn't really need this state, since the few places that check can infer it
from the RFHM's pending deletion list.

     // The RFH is swapped out and stored inside a RenderFrameProxyHost, being
-    // used as a placeholder to allow cross-process communication.
+    // used as a placeholder to allow cross-process communication.  Only main
+    // frames can enter this state.
     STATE_SWAPPED_OUT,
   };
   // Helper function to determine whether the RFH state should contribute to the
@@ -214,11 +210,12 @@ class CONTENT_EXPORT RenderFrameHostImpl
       const TransitionLayerData& transition_data);
 
   // Tells the renderer that this RenderFrame is being swapped out for one in a
-  // different renderer process.  It should run its unload handler, move to
-  // a blank document and create a RenderFrameProxy to replace the RenderFrame.
-  // The renderer should preserve the Proxy object until it exits, in case we
-  // come back.  The renderer can exit if it has no other active RenderFrames,
-  // but not until WasSwappedOut is called (when it is no longer visible).
+  // different renderer process.  It should run its unload handler and move to
+  // a blank document.  If |proxy| is not null, it should also create a
+  // RenderFrameProxy to replace the RenderFrame. The renderer should preserve
+  // the RenderFrameProxy object until it exits, in case we come back.  The
+  // renderer can exit if it has no other active RenderFrames, but not until
+  // WasSwappedOut is called (when it is no longer visible).

[nasko, 2014/10/02 16:04:02]

nit: the "(when it is no longer visible)" part is stale, isn't it? Since we run
the unload handler asynchronously, we show the new frame at CommitPending, which
doesn't depend on WasSwappedOut anymore.

[Charlie Reis, 2014/10/02 19:41:19]

Yep, good catch.

   void SwapOut(RenderFrameProxyHost* proxy);
 
   bool is_waiting_for_beforeunload_ack() const {
@@ -239,10 +236,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // The current state of this RFH.
   RenderFrameHostImplState rfh_state() const { return rfh_state_; }
 
-  // Set |this| as pending shutdown. |on_swap_out| will be called
-  // when the SwapOutACK is received, or when the unload timer times out.
-  void SetPendingShutdown(const base::Closure& on_swap_out);
-
   // Sends the given navigation message. Use this rather than sending it
   // yourself since this does the internal bookkeeping described below. This
   // function takes ownership of the provided message pointer.
@@ -552,10 +545,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // SiteInstance.
   scoped_ptr<TimeoutMonitor> swapout_event_monitor_timeout_;
 
-  // Called after receiving the SwapOutACK when the RFH is in the pending
-  // shutdown state. Also called if the unload timer times out.
-  base::Closure pending_shutdown_on_swap_out_;
-
   ServiceRegistryImpl service_registry_;
 
   scoped_ptr<BrowserAccessibilityManager> browser_accessibility_manager_;