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

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 186 matching lines @@
 
 void FrameTree::CreateProxiesForSiteInstance(
     FrameTreeNode* source,
     SiteInstance* site_instance) {
   // Create the swapped out RVH for the new SiteInstance. This will create
   // a top-level swapped out RFH as well, which will then be wrapped by a
   // RenderFrameProxyHost.
   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);
     }
   }
 
   scoped_refptr<SiteInstance> instance(site_instance);
 
   // Proxies are created in the FrameTree in response to a node navigating to a
   // new SiteInstance. Since |source|'s navigation will replace the currently
   // loaded document, the entire subtree under |source| will be removed.
@@ skipping 39 matching lines @@
                                                     bool hidden) {
   DCHECK(main_frame_routing_id != MSG_ROUTING_NONE);
   RenderViewHostMap::iterator iter =
       render_view_host_map_.find(site_instance->GetId());
   if (iter != render_view_host_map_.end()) {
     // If a RenderViewHost's main frame is pending deletion for this
     // |site_instance|, put it in the map of RenderViewHosts pending shutdown.
     // 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));
       render_view_host_map_.erase(iter);
     } else {
       return iter->second;
     }
   }
   RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
       RenderViewHostFactory::Create(site_instance,
@@ skipping 67 matching lines @@
         rvh->Shutdown();
         render_view_host_pending_shutdown_map_.erase(multi_iter);
       }
       break;
     }
     CHECK(render_view_host_found);
   }
 }
 
 }  // namespace content