Disallow was_within_same_page = true for a cross-process navigation

content/browser/frame_host/navigator_impl.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/navigator_impl.h"
 
 #include <utility>
 
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
@@ skipping 437 matching lines @@
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
   FrameTree* frame_tree = render_frame_host->frame_tree_node()->frame_tree();
   bool oopifs_possible = SiteIsolationPolicy::AreCrossProcessFramesPossible();
 
   bool has_embedded_credentials =
       params.url.has_username() || params.url.has_password();
   UMA_HISTOGRAM_BOOLEAN("Navigation.FrameHasEmbeddedCredentials",
                         has_embedded_credentials);
 
+  RenderFrameHostManager* manager = NULL;
+  if (oopifs_possible)
+    manager = render_frame_host->frame_tree_node()->render_manager();
+  else if (ui::PageTransitionIsMainFrame(params.transition))
+    manager = frame_tree->root()->render_manager();
+  bool will_swap = manager &&
+                   (render_frame_host == manager->pending_frame_host() ||
+                    render_frame_host == manager->speculative_frame_host());
+
   bool is_navigation_within_page = controller_->IsURLInPageNavigation(
-      params.url, params.was_within_same_page, render_frame_host);
+      params.url, params.was_within_same_page, render_frame_host, will_swap);

[Charlie Reis, 2016/02/26 20:11:58]

What if we left IsURLInPageNavigation as is and just verified that
is_navigation_within_page can only be true if render_frame_host ==
render_frame_host->frame_tree_node()->render_manager()->current_host()?  If the
RFH isn't the current one, we should kill it here (and proceed with committing /
showing the sad tab).

[gzobqq, 2016/02/29 13:36:28]

I agree, this is cleaner.

   if (ui::PageTransitionIsMainFrame(params.transition)) {
     if (delegate_) {
       // When overscroll navigation gesture is enabled, a screenshot of the page
       // in its current state is taken so that it can be used during the
       // nav-gesture. It is necessary to take the screenshot here, before
       // calling RenderFrameHostManager::DidNavigateMainFrame, because that can
       // change WebContents::GetRenderViewHost to return the new host, instead
       // of the one that may have just been swapped out.
       if (delegate_->CanOverscrollContent()) {
         // Don't take screenshots if we are staying on the same page. We want
@@ skipping 26 matching lines @@
 
   render_frame_host->frame_tree_node()->SetEnforceStrictMixedContentChecking(
       params.should_enforce_strict_mixed_content_checking);
 
   // When using --site-per-process, we notify the RFHM for all navigations,
   // not just main frame navigations.
   if (oopifs_possible) {
     FrameTreeNode* frame = render_frame_host->frame_tree_node();
     frame->render_manager()->DidNavigateFrame(
         render_frame_host, params.gesture == NavigationGestureUser);
   }

[Charlie Reis, 2016/02/26 20:11:58]

Should we also verify that |render_frame_host| is the current RFH after calling
RFHM::DidNavigateFrame, and kill it if not?

[gzobqq, 2016/02/29 13:36:28]

From my reading of DidNavigateFrame, it doesn't seem to change
current_frame_host() if render_frame_host == current_frame_host(). Shall I add a
double check?

 
   // Update the site of the SiteInstance if it doesn't have one yet, unless
   // assigning a site is not necessary for this URL.  In that case, the
   // SiteInstance can still be considered unused until a navigation to a real
   // page.
   SiteInstanceImpl* site_instance = render_frame_host->GetSiteInstance();
   if (!site_instance->HasSite() &&
       ShouldAssignSiteForURL(params.url)) {
     site_instance->SetSite(params.url);
   }
@@ skipping 548 matching lines @@
       entry->set_should_replace_entry(pending_entry->should_replace_entry());
       entry->SetRedirectChain(pending_entry->GetRedirectChain());
     }
     controller_->SetPendingEntry(std::move(entry));
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content