Move the RenderProcessGone IPC from RenderViewHost to RenderFrameHost

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 5878180ae16a4662195f58f2e417b14557f21ec0..14e71aee036bdd7da6210363fe3e8c15f8cc198e 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -365,6 +365,9 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
     IPC_MESSAGE_HANDLER(AccessibilityHostMsg_FindInPageResult,
                         OnAccessibilityFindInPageResult)
     IPC_MESSAGE_HANDLER(FrameHostMsg_ToggleFullscreen, OnToggleFullscreen)
+    // The following message is synthetic and doesn't come from RenderFrame, but
+    // from RenderProcessHost.
+    IPC_MESSAGE_HANDLER(FrameHostMsg_RenderProcessGone, OnRenderProcessGone)
 #if defined(OS_MACOSX) || defined(OS_ANDROID)
     IPC_MESSAGE_HANDLER(FrameHostMsg_ShowPopup, OnShowPopup)
     IPC_MESSAGE_HANDLER(FrameHostMsg_HidePopup, OnHidePopup)
@@ -995,6 +998,28 @@ void RenderFrameHostImpl::OnSwapOutACK() {
   OnSwappedOut();
 }
 
+void RenderFrameHostImpl::OnRenderProcessGone(int status, int exit_code) {
+  // Reset frame tree state associated with this process.  This must happen
+  // before RenderViewTerminated because observers expect the subframes of any
+  // affected frames to be cleared first.
+  frame_tree_node_->frame_tree()->RenderProcessGone(render_view_host_);

[Charlie Reis, 2015/02/03 01:06:35]

Does OnRenderProcessGone get called once per frame?  If so, should we really be
calling this multiple times?

[nasko, 2015/02/04 16:51:31]

It is indeed called once per frame. Ideally it will be called one per process,
but I don't know of a clean way to do it. It should be the case that any
subsequent calls for frames in the same process will just execute without making
any further changes to the FrameTree.

+
+  if (frame_tree_node_->IsMainFrame()) {
+    // Keep the termination status so we can get at it later when we
+    // need to know why it died.
+    render_view_host_->render_view_termination_status_ =

[Charlie Reis, 2015/02/03 01:06:35]

Sanity check: This used to be set before the FrameTree::RenderProcessGone call. 
Can you make sure it's not accessed from within there if we're going to reorder
it?

[nasko, 2015/02/04 16:51:31]

I don't think I can reasonably guarantee this. Therefore I'm splitting the if
statement to preserve the ordering.

+        static_cast<base::TerminationStatus>(status);
+
+    // Our base class RenderWidgetHost needs to reset some stuff.

[Charlie Reis, 2015/02/03 01:06:35]

nit: Not our base class anymore.

[nasko, 2015/02/04 16:51:31]

Done.

+    render_view_host_->RendererExited(
+        render_view_host_->render_view_termination_status_, exit_code);
+
+    render_view_host_->delegate_->RenderViewTerminated(
+        render_view_host_, static_cast<base::TerminationStatus>(status),
+        exit_code);
+  }
+}
+
 void RenderFrameHostImpl::OnSwappedOut() {
   // Ignore spurious swap out ack.
   if (rfh_state_ != STATE_PENDING_SWAP_OUT)