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

content/browser/frame_host/frame_tree.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/browser/frame_host/frame_tree.h"
 
 #include <queue>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 25 matching lines @@
                              FrameTreeNode** out_node,
                              FrameTreeNode* node) {
   if (node->frame_id() == frame_id) {
     *out_node = node;
     // Terminate iteration once the node has been found.
     return false;
   }
   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,
                      RenderFrameHostDelegate* render_frame_delegate,
                      RenderViewHostDelegate* render_view_delegate,
                      RenderWidgetHostDelegate* render_widget_delegate,
                      RenderFrameHostManager::Delegate* manager_delegate)
     : render_frame_delegate_(render_frame_delegate),
       render_view_delegate_(render_view_delegate),
       render_widget_delegate_(render_widget_delegate),
@@ skipping 90 matching lines @@
   // TODO(ajwong): Should the renderer be killed here? Would there be a race on
   // shutdown that might make this case possible?
   if (!node)
     return;
 
   if (node)
     node->set_current_url(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 {
   return root_->current_frame_host();
 }
 
 void FrameTree::SetFrameRemoveListener(
     const base::Callback<void(RenderViewHostImpl*, int64)>& on_frame_removed) {
   on_frame_removed_ = on_frame_removed;
 }
@@ skipping 101 matching lines @@
   }
 }
 
 FrameTreeNode* FrameTree::FindByFrameID(int64 frame_id) {
   FrameTreeNode* node = NULL;
   ForEach(base::Bind(&FrameTreeNodeForFrameId, frame_id, &node));
   return node;
 }
 
 }  // namespace content