With --site-per-process, avoid a crash when a subframe process goes away.

content/browser/frame_host/frame_tree.cc

Index: content/browser/frame_host/frame_tree.cc
diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc
index c135da80d340fb2c3a90bdb3c57d138cc2fbfd91..e890b3e2e9223fc383af8be6af4fe145d1d529f6 100644
--- a/content/browser/frame_host/frame_tree.cc
+++ b/content/browser/frame_host/frame_tree.cc
@@ -43,6 +43,16 @@ bool FrameTreeNodeForFrameId(int64 frame_id,
   return true;
 }
 
+// Iterate over the FrameTree to reset any node affected by the loss of the
+// given RenderViewHost's process.
+bool ResetNodesForNewProcess(RenderViewHost* render_view_host,

[nasko, 2014/02/11 04:49:16]

Shouldn't we be passing the process that is gone? Adding new code that uses RVH
seems wrong to me. We can pass the RPH that has crashed and check each frame's
process for a match, can't we?

[Charlie Reis, 2014/02/12 00:27:57]

I've added a TODO for that.  For now, that just adds unnecessary work for both
the caller and each step of the tree traversal, since the only way to get to the
RPH is by asking the RVH.  I agree that we can cut out RVH entirely once RFH
knows its process directly.

+                             FrameTreeNode** out_node,

[awong, 2014/02/11 18:24:16]

Why bother with out_node at all?

[Charlie Reis, 2014/02/12 00:27:57]

base::Bind is magic.  Done.

+                             FrameTreeNode* node) {
+  if (render_view_host == node->current_frame_host()->render_view_host())
+    node->ResetForNewProcess();
+  return true;
+}
+
 }  // namespace
 
 FrameTree::FrameTree(Navigator* navigator,
@@ -153,7 +163,18 @@ void FrameTree::SetFrameUrl(int64 frame_id, const GURL& url) {
 }
 
 void FrameTree::ResetForMainFrameSwap() {
-  return root_->ResetForMainFrameSwap();
+  root_->ResetForNewProcess();
+}
+
+void FrameTree::RenderProcessGone(RenderViewHost* render_view_host) {

[nasko, 2014/02/11 04:49:16]

Again, it will be better with RPH parameter.

[Charlie Reis, 2014/02/12 00:27:57]

Then we're just calling RenderViewHostImpl::GetProcess via
RenderFrameHostImpl::GetProcess at each node, as well as the call site of this
function.  I think it makes sense when RFH starts to know its own process.

+  // Walk the full tree looking for nodes that may be affected.  Once a frame
+  // crashes, all of its child FrameTreeNodes go away.
+  // Note that the helper function may call ResetForNewProcess on a node, which

[nasko, 2014/02/11 04:49:16]

If we are now putting explicit dependency on this order of operation, it will be
useful to put a comment in ForEach, to ensure we don't change it and break
things.

[Charlie Reis, 2014/02/12 00:27:57]

Done.

+  // clears its children before we iterate over them.  That's ok, because
+  // ForEach does not add a node's children to the queue until after visiting
+  // the node itself.
+  FrameTreeNode* node = NULL;
+  ForEach(base::Bind(&ResetNodesForNewProcess, render_view_host, &node));
 }
 
 RenderFrameHostImpl* FrameTree::GetMainFrame() const {