PlzNavigate: implement RequestNavigation in the no live renderer case

content/browser/frame_host/render_frame_host_manager.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/render_frame_host_manager.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 21 matching lines @@
 #include "content/common/view_messages.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
+#include "net/base/load_flags.h"
 
 namespace content {
 
+namespace {
+
+FrameHostMsg_BeginNavigation_Params BeginNavigationFromNavigate(
+    const FrameMsg_Navigate_Params& navigate_params) {
+  FrameHostMsg_BeginNavigation_Params begin_navigation_params;
+  begin_navigation_params.method = navigate_params.is_post ?
+      "POST" : "GET";
+  begin_navigation_params.url = navigate_params.url;
+  begin_navigation_params.referrer = navigate_params.referrer.url;
+  begin_navigation_params.referrer_policy = navigate_params.referrer.policy;
+
+  // TODO(clamy): This should be modified to take into account caching policy
+  // requirements (eg for POST reloads).
+  begin_navigation_params.load_flags =
+      net::LOAD_NORMAL | net::LOAD_ENABLE_LOAD_TIMING;
+
+  // TODO(clamy): Post data from the browser should be put in the request body.
+
+  begin_navigation_params.has_user_gesture = false;
+  begin_navigation_params.transition_type = navigate_params.transition;
+  begin_navigation_params.should_replace_current_entry =
+      navigate_params.should_replace_current_entry;
+  begin_navigation_params.allow_download =
+      navigate_params.allow_download;
+  return begin_navigation_params;
+}
+
+}  // namespace
+
 RenderFrameHostManager::PendingNavigationParams::PendingNavigationParams(
     const GlobalRequestID& global_request_id,
     scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request,
     const std::vector<GURL>& transfer_url_chain,
     Referrer referrer,
     PageTransition page_transition,
     int render_frame_id,
     bool should_replace_current_entry)
     : global_request_id(global_request_id),
       cross_site_transferring_request(cross_site_transferring_request.Pass()),
@@ skipping 118 matching lines @@
 }
 
 RenderFrameHostImpl* RenderFrameHostManager::Navigate(
     const NavigationEntryImpl& entry) {
   TRACE_EVENT0("browser", "RenderFrameHostManager:Navigate");
   // Create a pending RenderFrameHost to use for the navigation.
   RenderFrameHostImpl* dest_render_frame_host = UpdateStateForNavigate(entry);
   if (!dest_render_frame_host)
     return NULL;  // We weren't able to create a pending render frame host.
 
-  // If the current render_frame_host_ isn't live, we should create it so
-  // that we don't show a sad tab while the dest_render_frame_host fetches
-  // its first page.  (Bug 1145340)
-  if (dest_render_frame_host != render_frame_host_ &&
-      !render_frame_host_->render_view_host()->IsRenderViewLive()) {
-    // Note: we don't call InitRenderView here because we are navigating away
-    // soon anyway, and we don't have the NavigationEntry for this host.
-    delegate_->CreateRenderViewForRenderManager(
-        render_frame_host_->render_view_host(), MSG_ROUTING_NONE,
-        MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame());
-  }
-
-  // If the renderer crashed, then try to create a new one to satisfy this
-  // navigation request.
-  if (!dest_render_frame_host->render_view_host()->IsRenderViewLive()) {
-    // Recreate the opener chain.
-    int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager(
-        dest_render_frame_host->GetSiteInstance());
-    if (!InitRenderView(dest_render_frame_host->render_view_host(),
-                        opener_route_id,
-                        MSG_ROUTING_NONE,
-                        frame_tree_node_->IsMainFrame()))
-      return NULL;
-
-    // Now that we've created a new renderer, be sure to hide it if it isn't
-    // our primary one.  Otherwise, we might crash if we try to call Show()
-    // on it later.
-    if (dest_render_frame_host != render_frame_host_ &&
-        dest_render_frame_host->render_view_host()->GetView()) {
-      dest_render_frame_host->render_view_host()->GetView()->Hide();
-    } else {
-      // Notify here as we won't be calling CommitPending (which does the
-      // notify).
-      delegate_->NotifySwappedFromRenderManager(
-          NULL, render_frame_host_.get(), frame_tree_node_->IsMainFrame());
-    }
-  }
+  // Initialize the destination RFH and the current RFH if they are not live.

[nasko, 2014/07/17 12:05:08]

If they are not alive, then they will be the same, right? The comment implies
that they can be different.

[clamy, 2014/07/17 13:07:53]

What if the current renderer crashed and we navigate to a brand new different
site, do we reuse the RFH in that case?

[nasko, 2014/07/21 13:02:05]

No, we don't. I was thinking initial creation only and didn't take into account
the crashed renderer.

+  if (!InitRenderFrameHostsBeforeNavigation(dest_render_frame_host))
+    return NULL;
 
   // If entry includes the request ID of a request that is being transferred,
   // the destination render frame will take ownership, so release ownership of
   // the request.
   if (pending_nav_params_ &&
       pending_nav_params_->global_request_id ==
           entry.transferred_global_request_id()) {
     pending_nav_params_->cross_site_transferring_request->ReleaseRequest();
   }
 
   return dest_render_frame_host;
 }
 
+bool RenderFrameHostManager::RequestNavigation(
+    const NavigationEntryImpl& entry,
+    const FrameMsg_Navigate_Params& navigate_params) {
+  if (render_frame_host_->render_view_host()->IsRenderViewLive())

[nasko, 2014/07/17 12:05:08]

Let's put a TODO to put IsLive on RenderFrameHost, as it doesn't make sense to
go to the RVH long term.

[clamy, 2014/07/17 13:07:53]

Done.

+    // TODO(clamy): send a RequestNavigation IPC.
+    return true;
+
+  // The navigation request is sent directly to the IO thread.
+  OnBeginNavigation(BeginNavigationFromNavigate(navigate_params));

[nasko, 2014/07/17 12:05:08]

There is a comment in OnBeginNavigation to create the speculative RFH. You are
doing it below. One of the two has to be corrected.

[clamy, 2014/07/17 13:07:53]

The comment in OnBeginNavigation had more to do with creating a RFH
speculatively in case we realize we have to do a cross-site navigation and we
don't have a proper renderer for that. I guess I could move the RFH creation
part below into OnBeginNavigation, so that we only have speculative RFH spawning
in one place.

+
+  // Create a new RFH for the navigation if necessary.
+  RenderFrameHostImpl* dest_render_frame_host = render_frame_host_.get();
+  SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
+  SiteInstance* new_instance = GetSiteInstanceForNavigation(entry);
+  if (current_instance != new_instance) {
+    // Ensure that we have created RFHs for the new RFH's opener chain if
+    // we are staying in the same BrowsingInstance. This allows the pending RFH
+    // to send cross-process script calls to its opener(s).
+    int opener_route_id = MSG_ROUTING_NONE;
+    if (new_instance->IsRelatedSiteInstance(current_instance)) {
+      opener_route_id =
+          delegate_->CreateOpenerRenderViewsForRenderManager(new_instance);
+    }
+
+    // Create a non-swapped-out speculative RFH with the given opener.
+    int route_id = CreateRenderFrame(new_instance, opener_route_id, false,
+                                     delegate_->IsHidden(), true);
+    if (route_id == MSG_ROUTING_NONE)
+      return false;
+    dest_render_frame_host = speculative_render_frame_host_.get();
+  }
+  return InitRenderFrameHostsBeforeNavigation(dest_render_frame_host);

[nasko, 2014/07/17 12:05:08]

Do we need to init in the case where we don't create a new RFH? 

[clamy, 2014/07/17 13:07:53]

We are in the case where the current RFH is not live, so if we don't create a
new one, we are likely to use the not live current_render_frame_host_ when the
navigation commits. I think it should be initialized, but I may be mistaken.

[nasko, 2014/07/21 13:02:05]

I don't think we need a live current renderer to start the new navigation, but
handling that case might just make the code more complex, which I'm not a fan
of.

[clamy, 2014/07/21 13:28:17]

So should I let the initialization in place?

+}
+
 void RenderFrameHostManager::Stop() {
   render_frame_host_->render_view_host()->Stop();
 
   // If we are cross-navigating, we should stop the pending renderers.  This
   // will lead to a DidFailProvisionalLoad, which will properly destroy them.
   if (cross_navigation_pending_) {
     pending_render_frame_host_->render_view_host()->Send(new ViewMsg_Stop(
         pending_render_frame_host_->render_view_host()->GetRoutingID()));
   }
 }
@@ skipping 694 matching lines @@
                                                      frame_tree_node_,
                                                      frame_routing_id,
                                                      swapped_out).release());
   return render_frame_host.Pass();
 }
 
 int RenderFrameHostManager::CreateRenderFrame(
     SiteInstance* instance,
     int opener_route_id,
     bool swapped_out,
-    bool hidden) {
+    bool hidden,
+    bool is_speculative) {
   CHECK(instance);
   DCHECK(!swapped_out || hidden); // Swapped out views should always be hidden.

[nasko, 2014/07/17 12:05:08]

is_speculative should never be true when swapped_out is true, right? If yes,
then let's put a DCHECK to enforce it.

[clamy, 2014/07/17 13:07:53]

Done.

 
   scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
   RenderFrameHostImpl* frame_to_announce = NULL;
   int routing_id = MSG_ROUTING_NONE;
 
   // We are creating a pending or swapped out RFH here.  We should never create
   // it in the same SiteInstance as our current RFH.
   CHECK_NE(render_frame_host_->GetSiteInstance(), instance);
 
   // Check if we've already created an RFH for this SiteInstance.  If so, try
@@ skipping 53 matching lines @@
       // Don't show the main frame's view until we get a DidNavigate from it.
       render_view_host->GetView()->Hide();
     } else if (!swapped_out && pending_render_frame_host_) {
       CancelPending();
     }
     routing_id = render_view_host->GetRoutingID();
     frame_to_announce = new_render_frame_host.get();
   }
 
   // Use this as our new pending RFH if it isn't swapped out.
-  if (!swapped_out)
-    pending_render_frame_host_ = new_render_frame_host.Pass();
+  if (!swapped_out) {
+    if (is_speculative)
+      speculative_render_frame_host_ = new_render_frame_host.Pass();
+    else
+      pending_render_frame_host_ = new_render_frame_host.Pass();
+  }
 
   // If a brand new RFH was created, announce it to observers.
   if (frame_to_announce)
     render_frame_delegate_->RenderFrameCreated(frame_to_announce);
 
   return routing_id;
 }
 
 bool RenderFrameHostManager::InitRenderView(RenderViewHost* render_view_host,
                                             int opener_route_id,
@@ skipping 16 matching lines @@
     if (!rvh_impl->IsSwappedOut()) {
       CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
                 render_view_host->GetProcess()->GetID()));
     }
   }
 
   return delegate_->CreateRenderViewForRenderManager(
       render_view_host, opener_route_id, proxy_routing_id, for_main_frame);
 }
 
+bool RenderFrameHostManager::InitRenderFrameHostsBeforeNavigation(
+    RenderFrameHostImpl* dest_render_frame_host) {
+  // If the current render_frame_host_ isn't live, we should create it so
+  // that we don't show a sad tab while the dest_render_frame_host fetches
+  // its first page.  (Bug 1145340)

[nasko, 2014/07/17 12:05:08]

nit: convert this to a crbug link.

[clamy, 2014/07/17 13:07:53]

Done.

+  if (dest_render_frame_host != render_frame_host_ &&
+      !render_frame_host_->render_view_host()->IsRenderViewLive()) {
+    // Note: we don't call InitRenderView here because we are navigating away
+    // soon anyway, and we don't have the NavigationEntry for this host.
+    delegate_->CreateRenderViewForRenderManager(
+        render_frame_host_->render_view_host(), MSG_ROUTING_NONE,
+        MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame());
+  }
+
+  // If the renderer crashed, then try to create a new one to satisfy this
+  // navigation request.
+  if (!dest_render_frame_host->render_view_host()->IsRenderViewLive()) {
+    // Recreate the opener chain.
+    int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager(
+        dest_render_frame_host->GetSiteInstance());
+    if (!InitRenderView(dest_render_frame_host->render_view_host(),
+                        opener_route_id,
+                        MSG_ROUTING_NONE,
+                        frame_tree_node_->IsMainFrame()))
+      return false;
+
+    // Now that we've created a new renderer, be sure to hide it if it isn't
+    // our primary one.  Otherwise, we might crash if we try to call Show()
+    // on it later.
+    if (dest_render_frame_host != render_frame_host_ &&
+        dest_render_frame_host->render_view_host()->GetView()) {
+      dest_render_frame_host->render_view_host()->GetView()->Hide();
+    } else {
+      // Notify here as we won't be calling CommitPending (which does the
+      // notify).
+      delegate_->NotifySwappedFromRenderManager(
+          NULL, render_frame_host_.get(), frame_tree_node_->IsMainFrame());
+    }
+  }
+  return true;
+}
+
+SiteInstance* RenderFrameHostManager::GetSiteInstanceForNavigation(
+    const NavigationEntryImpl& entry) {
+  SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
+  SiteInstance* new_instance = current_instance;
+
+  // We do not currently swap processes for navigations in webview tag guests.
+  bool is_guest_scheme = current_instance->GetSiteURL().SchemeIs(kGuestScheme);
+
+  // Determine if we need a new BrowsingInstance for this entry.  If true, this
+  // implies that it will get a new SiteInstance (and likely process), and that
+  // other tabs in the current BrowsingInstance will be unable to script it.
+  // This is used for cases that require a process swap even in the
+  // process-per-tab model, such as WebUI pages.
+  const NavigationEntry* current_entry =
+      delegate_->GetLastCommittedNavigationEntryForRenderManager();
+  bool force_swap = !is_guest_scheme &&
+      ShouldSwapBrowsingInstancesForNavigation(current_entry, &entry);
+  if (!is_guest_scheme && (ShouldTransitionCrossSite() || force_swap))
+    new_instance = GetSiteInstanceForEntry(entry, current_instance, force_swap);
+
+  // If force_swap is true, we must use a different SiteInstance.  If we didn't,
+  // we would have two RenderFrameHosts in the same SiteInstance and the same
+  // frame, resulting in page_id conflicts for their NavigationEntries.
+  if (force_swap)
+    CHECK_NE(new_instance, current_instance);
+
+  return new_instance;
+}
+
 void RenderFrameHostManager::CommitPending() {
   // First check whether we're going to want to focus the location bar after
   // this commit.  We do this now because the navigation hasn't formally
   // committed yet, so if we've already cleared |pending_web_ui_| the call chain
   // this triggers won't be able to figure out what's going on.
   bool will_focus_location_bar = delegate_->FocusLocationBarByDefault();
 
   // We expect SwapOutOldPage to have canceled any modal dialogs and told the
   // renderer to suppress any further dialogs until it is swapped out.  However,
   // crash reports indicate that it's still possible for modal dialogs to exist
@@ skipping 188 matching lines @@
   if (cross_navigation_pending_) {
     if (pending_render_frame_host_)
       CancelPending();
     cross_navigation_pending_ = false;
   }
 
   // render_frame_host_'s SiteInstance and new_instance will not be deleted
   // before the end of this method, so we don't have to worry about their ref
   // counts dropping to zero.
   SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
-  SiteInstance* new_instance = current_instance;
-
-  // We do not currently swap processes for navigations in webview tag guests.
-  bool is_guest_scheme = current_instance->GetSiteURL().SchemeIs(kGuestScheme);
-
-  // Determine if we need a new BrowsingInstance for this entry.  If true, this
-  // implies that it will get a new SiteInstance (and likely process), and that
-  // other tabs in the current BrowsingInstance will be unable to script it.
-  // This is used for cases that require a process swap even in the
-  // process-per-tab model, such as WebUI pages.
+  SiteInstance* new_instance = GetSiteInstanceForNavigation(entry);
   const NavigationEntry* current_entry =
       delegate_->GetLastCommittedNavigationEntryForRenderManager();
-  bool force_swap = !is_guest_scheme &&
-      ShouldSwapBrowsingInstancesForNavigation(current_entry, &entry);
-  if (!is_guest_scheme && (ShouldTransitionCrossSite() || force_swap))
-    new_instance = GetSiteInstanceForEntry(entry, current_instance, force_swap);
-
-  // If force_swap is true, we must use a different SiteInstance.  If we didn't,
-  // we would have two RenderFrameHosts in the same SiteInstance and the same
-  // frame, resulting in page_id conflicts for their NavigationEntries.
-  if (force_swap)
-    CHECK_NE(new_instance, current_instance);
 
   if (new_instance != current_instance) {
     // New SiteInstance: create a pending RFH to navigate.
     DCHECK(!cross_navigation_pending_);
 
     // This will possibly create (set to NULL) a Web UI object for the pending
     // page. We'll use this later to give the page special access. This must
     // happen before the new renderer is created below so it will get bindings.
     // It must also happen after the above conditional call to CancelPending(),
     // otherwise CancelPending may clear the pending_web_ui_ and the page will
     // not have its bindings set appropriately.
     SetPendingWebUI(entry);
 
     // Ensure that we have created RFHs for the new RFH's opener chain if
     // we are staying in the same BrowsingInstance. This allows the pending RFH
     // to send cross-process script calls to its opener(s).
     int opener_route_id = MSG_ROUTING_NONE;
     if (new_instance->IsRelatedSiteInstance(current_instance)) {
       opener_route_id =
           delegate_->CreateOpenerRenderViewsForRenderManager(new_instance);
     }
 
     // Create a non-swapped-out pending RFH with the given opener and navigate
     // it.
     int route_id = CreateRenderFrame(new_instance, opener_route_id, false,
-                                     delegate_->IsHidden());
+                                     delegate_->IsHidden(), false);
     if (route_id == MSG_ROUTING_NONE)
       return NULL;
 
     // Check if our current RFH is live before we set up a transition.
     if (!render_frame_host_->render_view_host()->IsRenderViewLive()) {
       if (!cross_navigation_pending_) {
         // The current RFH is not live.  There's no reason to sit around with a
         // sad tab or a newly created RFH while we wait for the pending RFH to
         // navigate.  Just switch to the pending RFH now and go back to non
         // cross-navigating (Note that we don't care about on{before}unload
@@ skipping 179 matching lines @@
     SiteInstance* instance) const {
   RenderFrameProxyHostMap::const_iterator iter =
       proxy_hosts_.find(instance->GetId());
   if (iter != proxy_hosts_.end())
     return iter->second;
 
   return NULL;
 }
 
 }  // namespace content