PlzNavigate: fix issue preventing navigations to WebUIs

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 <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 717 matching lines @@
     pending_render_frame_host_->ClearAllWebUI();
   // PlzNavigate
   if (speculative_render_frame_host_)
     speculative_render_frame_host_->ClearAllWebUI();
 }
 
 // PlzNavigate
 void RenderFrameHostManager::DidCreateNavigationRequest(
     NavigationRequest* request) {
   CHECK(IsBrowserSideNavigationEnabled());
-  RenderFrameHostImpl* dest_rfh = GetFrameHostForNavigation(*request);
+  RenderFrameHostImpl* dest_rfh = GetFrameHostForNavigation(request);
   DCHECK(dest_rfh);
-  request->set_associated_site_instance_type(
-      dest_rfh == render_frame_host_.get()
-          ? NavigationRequest::AssociatedSiteInstanceType::CURRENT
-          : NavigationRequest::AssociatedSiteInstanceType::SPECULATIVE);

[Charlie Reis, 2016/05/23 17:26:40]

Just curious, what was the reason this needed to be moved inside
GetFrameHostForNavigation?  Was there a path that wasn't going through
DidCreateNavigationRequest?

[clamy, 2016/05/24 11:18:32]

Yes this was a TODO to move it to GetFrameHostForNavigation.
GetFrameHostForNavigation has 2 other call sites (at commit time).

 }
 
 // PlzNavigate
 RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
-    const NavigationRequest& request) {
+    NavigationRequest* request) {
   CHECK(IsBrowserSideNavigationEnabled());
 
   SiteInstance* current_site_instance = render_frame_host_->GetSiteInstance();
 
   SiteInstance* candidate_site_instance =
       speculative_render_frame_host_
           ? speculative_render_frame_host_->GetSiteInstance()
           : nullptr;
 
   scoped_refptr<SiteInstance> dest_site_instance = GetSiteInstanceForNavigation(
-      request.common_params().url, request.source_site_instance(),
-      request.dest_site_instance(), candidate_site_instance,
-      request.common_params().transition,
-      request.restore_type() != NavigationEntryImpl::RESTORE_NONE,
-      request.is_view_source());
+      request->common_params().url, request->source_site_instance(),
+      request->dest_site_instance(), candidate_site_instance,
+      request->common_params().transition,
+      request->restore_type() != NavigationEntryImpl::RESTORE_NONE,
+      request->is_view_source());
 
   // The appropriate RenderFrameHost to commit the navigation.
   RenderFrameHostImpl* navigation_rfh = nullptr;
 
   bool notify_webui_of_rv_creation = false;
 
   // Reuse the current RenderFrameHost if its SiteInstance matches the
   // navigation's.
   bool no_renderer_swap = current_site_instance == dest_site_instance.get();
 
   if (frame_tree_node_->IsMainFrame()) {
     // Renderer-initiated main frame navigations that may require a
     // SiteInstance swap are sent to the browser via the OpenURL IPC and are
     // afterwards treated as browser-initiated navigations. NavigationRequests
     // marked as renderer-initiated are created by receiving a BeginNavigation
     // IPC, and will then proceed in the same renderer. In site-per-process
     // mode, it is possible for renderer-intiated navigations to be allowed to
     // go cross-process. Check it first.
     bool can_renderer_initiate_transfer =
         render_frame_host_->IsRenderFrameLive() &&
-        ShouldMakeNetworkRequestForURL(request.common_params().url) &&
+        ShouldMakeNetworkRequestForURL(request->common_params().url) &&
         IsRendererTransferNeededForNavigation(render_frame_host_.get(),
-                                              request.common_params().url);
+                                              request->common_params().url);
 
     no_renderer_swap |=
-        !request.browser_initiated() && !can_renderer_initiate_transfer;
+        !request->browser_initiated() && !can_renderer_initiate_transfer;
   } else {
     // Subframe navigations will use the current renderer, unless specifically
     // allowed to swap processes.
-    no_renderer_swap |= !CanSubframeSwapProcess(request.common_params().url,
-                                                request.source_site_instance(),
-                                                request.dest_site_instance());
+    no_renderer_swap |= !CanSubframeSwapProcess(request->common_params().url,
+                                                request->source_site_instance(),
+                                                request->dest_site_instance());
   }
 
   if (no_renderer_swap) {
     // GetFrameHostForNavigation will be called more than once during a
     // navigation (currently twice, on request and when it's about to commit in
     // the renderer). In the follow up calls an existing pending WebUI should
     // not be recreated if the URL didn't change. So instead of calling
     // CleanUpNavigation just discard the speculative RenderFrameHost if one
     // exists.
     if (speculative_render_frame_host_)
       DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());
 
-    UpdatePendingWebUIOnCurrentFrameHost(request.common_params().url,
-                                         request.bindings());
+    UpdatePendingWebUIOnCurrentFrameHost(request->common_params().url,
+                                         request->bindings());
 
     navigation_rfh = render_frame_host_.get();
 
     DCHECK(!speculative_render_frame_host_);
   } else {
     // If the current RenderFrameHost cannot be used a speculative one is
     // created with the SiteInstance for the current URL. If a speculative
     // RenderFrameHost already exists we try as much as possible to reuse it and
     // its associated WebUI.
 
     // Check if an existing speculative RenderFrameHost can be reused.
     if (!speculative_render_frame_host_ ||
         speculative_render_frame_host_->GetSiteInstance() !=
             dest_site_instance.get()) {
       // If a previous speculative RenderFrameHost didn't exist or if its
       // SiteInstance differs from the one for the current URL, a new one needs
       // to be created.
       CleanUpNavigation();
       bool success = CreateSpeculativeRenderFrameHost(current_site_instance,
                                                       dest_site_instance.get());
       DCHECK(success);
     }
     DCHECK(speculative_render_frame_host_);
 
     bool changed_web_ui = speculative_render_frame_host_->UpdatePendingWebUI(
-        request.common_params().url, request.bindings());
+        request->common_params().url, request->bindings());
     speculative_render_frame_host_->CommitPendingWebUI();
     DCHECK_EQ(GetNavigatingWebUI(), speculative_render_frame_host_->web_ui());
     notify_webui_of_rv_creation =
         changed_web_ui && speculative_render_frame_host_->web_ui();
 
     navigation_rfh = speculative_render_frame_host_.get();
 
     // Check if our current RFH is live.
     if (!render_frame_host_->IsRenderFrameLive()) {
       // The current RFH is not live.  There's no reason to sit around with a
@@ skipping 38 matching lines @@
     }
     DCHECK(navigation_rfh->IsRenderFrameLive());
   }
 
   // If a WebUI was created in a speculative RenderFrameHost or a new RenderView
   // was created then the WebUI never interacted with the RenderView. Notify
   // using RenderViewCreated.
   if (notify_webui_of_rv_creation && GetNavigatingWebUI())
     GetNavigatingWebUI()->RenderViewCreated(navigation_rfh->render_view_host());
 
+  request->set_associated_site_instance_type(
+      navigation_rfh == render_frame_host_.get()
+          ? NavigationRequest::AssociatedSiteInstanceType::CURRENT
+          : NavigationRequest::AssociatedSiteInstanceType::SPECULATIVE);
+
   return navigation_rfh;
 }
 
 // PlzNavigate
 void RenderFrameHostManager::CleanUpNavigation() {
   CHECK(IsBrowserSideNavigationEnabled());
   if (speculative_render_frame_host_) {
     bool was_loading = speculative_render_frame_host_->is_loading();
     DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());
     if (was_loading)
@@ skipping 1663 matching lines @@
                                              resolved_url)) {
     DCHECK(!dest_instance ||
            dest_instance == render_frame_host_->GetSiteInstance());
     return false;
   }
 
   return true;
 }
 
 }  // namespace content