Ensure that before creating proxy of site A, RVH of site A is initialized.

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 96890f09075d9e438e750271b3405f8ae3b2d80f..3934ec5d941084d0edecda7416665f0cd0169773 100644
--- a/content/browser/frame_host/frame_tree.cc
+++ b/content/browser/frame_host/frame_tree.cc
@@ -204,7 +204,7 @@ void FrameTree::CreateProxiesForSiteInstance(
   if (!source->IsMainFrame()) {
     RenderViewHostImpl* render_view_host =
         source->frame_tree()->GetRenderViewHost(site_instance);
-    if (!render_view_host) {
+    if (!render_view_host || !render_view_host->IsRenderViewLive()) {
       root()->render_manager()->CreateRenderFrame(

[Charlie Reis, 2014/12/12 19:18:31]

I took a closer look, and now I'm not sure this is the right fix.  We don't want
to create a new RVH+RFH if the previous ones are around but the process has
crashed.

Instead, we want to call RFHM::InitRenderView on the existing RVH if
!IsRenderViewLive.  (I'm guessing we don't want to call InitRenderFrame since
we'll be creating a proxy for it below.)

[nasko, 2014/12/12 22:25:01]

I agree we shouldn't be calling CreateRenderFrame. We already have the
browser-site host objects, we just need to create the renderer side ones.
InitRenderView is supposed to do this.

[lazyboy, 2014/12/16 07:10:04]

For context about uninitialized RVH:
  1
 / \
2   3
1 and 3 point to site A, 2 points to site B.
In process B, other than the proxies, we have
a) swapped out RVH_B,
b) swapped out sRFH_B, this is the "main_frame" for RVH_B. and
c) the non-swapped out frame for 2 say, RFH2.

[1] Now we kill the process of site B, this destroys sRFH_B along with
all the proxy hosts in process B,
but we still have RVH_B whose IsRenderViewLive==false, RFH2 pointing to 2.

Now if we try to navigate 3 to site B,
we see that we have the (swapped_out) RVH_B already, but the problem is
that its main_frame_routing_id is pointing to previously created
swapped_out sRFH_B's routing id which is no longer valid [2].

Does that sound right?
We do need to create this swapped out main render frame again [2]


(btw, trying to re-navigate 2 to site B again after killing process B in [1] is
a different story. We need to fix that separately).

[2] To fix this, one thing I tried is
FrameTree::CreateRenderViewHost() can return (existing)
RVH_B, but its main_frame needs to be updated to the point to the
render frame with |main_frame_routing_id| param of FT::CreateRenderViewHost().
This is shown in patchset #4

           site_instance, nullptr, MSG_ROUTING_NONE,
           CREATE_RF_SWAPPED_OUT | CREATE_RF_HIDDEN, nullptr);
@@ -265,8 +265,10 @@ RenderViewHostImpl* FrameTree::CreateRenderViewHost(SiteInstance* site_instance,
     // Otherwise return the existing RenderViewHost for the SiteInstance.
     RenderFrameHostImpl* main_frame = static_cast<RenderFrameHostImpl*>(
         iter->second->GetMainFrame());
-    if (main_frame->frame_tree_node()->render_manager()->IsPendingDeletion(
-            main_frame)) {
+    // |main_frame| is NULL when the RVH is uninitialized.

[nasko, 2014/12/12 22:25:01]

What sequence of events causes uninitialized RVH to be present in this map and
for this code to be reached?

[lazyboy, 2014/12/23 22:03:11]

Since we keep around the proxy (with swapped out rfh), this won't happen
anymore.
Before it was happening because the swapped out rfh wasn't there and
iter->second->main_frame_routing_id_ was pointing to that (deleted) swapped out
rfh's routing id.

+    if (!main_frame ||
+            main_frame->frame_tree_node()->render_manager()->IsPendingDeletion(
+                main_frame)) {
       render_view_host_pending_shutdown_map_.insert(
           std::pair<int, RenderViewHostImpl*>(site_instance->GetId(),
                                               iter->second));