PlzNavigate: Cancel ongoing history navigations in the browser when we receive a navigation request…

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/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 980 matching lines @@
     const BeginNavigationParams& begin_params) {
   // TODO(clamy): the url sent by the renderer should be validated with
   // FilterURL.
   // This is a renderer-initiated navigation.
   CHECK(IsBrowserSideNavigationEnabled());
   DCHECK(frame_tree_node);
 
   NavigationRequest* ongoing_navigation_request =
       frame_tree_node->navigation_request();
 
+  // Client redirects during the initial history navigation of a child frame
+  // should take precedence over the history navigation (despite being renderer-
+  // initiated).  See https://crbug.com/348447 and https://crbug.com/691168.
+  if (ongoing_navigation_request &&
+      ongoing_navigation_request->request_params()
+          .is_history_navigation_in_new_child) {
+    // Preemptively clear this local pointer before deleting the request.
+    ongoing_navigation_request = nullptr;
+    frame_tree_node->ResetNavigationRequest(false);

[jam, 2017/05/16 20:13:11]

btw I was tracing this code as part of
https://codereview.chromium.org/2889703002/, but I don't see it being hit for
the layout tests fixed here, or for any other layout or browser test. Is this
still needed?

[Charlie Reis, 2017/05/18 21:26:46]

Yep, it's still needed, but some races mean we aren't hitting it in tests.  We
should add a test for this branch of the race if we can-- I've filed
https://crbug.com/724271.

See https://codereview.chromium.org/2890613002/#msg15 for details.

+  }
+
   // The renderer-initiated navigation request is ignored iff a) there is an
   // ongoing request b) which is browser or user-initiated and c) the renderer
   // request is not user-initiated.
   if (ongoing_navigation_request &&
       (ongoing_navigation_request->browser_initiated() ||
        ongoing_navigation_request->begin_params().has_user_gesture) &&
       !begin_params.has_user_gesture) {
     RenderFrameHost* current_frame_host =
         frame_tree_node->render_manager()->current_frame_host();
     current_frame_host->Send(
@@ skipping 148 matching lines @@
                                       ReloadType reload_type,
                                       PreviewsState previews_state,
                                       bool is_same_document_history_load,
                                       bool is_history_navigation_in_new_child,
                                       base::TimeTicks navigation_start) {
   CHECK(IsBrowserSideNavigationEnabled());
   DCHECK(frame_tree_node);
 
   // This value must be set here because creating a NavigationRequest might
   // change the renderer live/non-live status and change this result.
+  // We don't want to dispatch a beforeunload handler if
+  // is_history_navigation_in_new_child is true. This indicates a newly created
+  // child frame which does not have a beforunload handler.
   bool should_dispatch_beforeunload =
       !is_same_document_history_load &&
+      !is_history_navigation_in_new_child &&
       frame_tree_node->current_frame_host()->ShouldDispatchBeforeUnload();
   FrameMsg_Navigate_Type::Value navigation_type = GetNavigationType(
       frame_tree_node->current_url(),  // old_url
       dest_url,                        // new_url
       reload_type,                     // reload_type
       entry,                           // entry
       frame_entry,                     // frame_entry
       is_same_document_history_load);  // is_same_document_history_load
   std::unique_ptr<NavigationRequest> scoped_request =
       NavigationRequest::CreateBrowserInitiated(
@@ skipping 140 matching lines @@
     if (navigation_handle)
       navigation_handle->update_entry_id_for_transfer(entry->GetUniqueID());
 
     controller_->SetPendingEntry(std::move(entry));
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content