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

content/browser/frame_host/navigator_impl.cc

Index: content/browser/frame_host/navigator_impl.cc
diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
index 997805938531dd69285fc32763d94d83c67f1a6c..8b116d5ffb1b0c792d8ca813288c463aae22c6ea 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -998,6 +998,17 @@ void NavigatorImpl::OnBeginNavigation(
   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);
+  }
+
   // 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.
@@ -1166,9 +1177,13 @@ void NavigatorImpl::RequestNavigation(FrameTreeNode* 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 an unload handler if

[Charlie Reis, 2017/02/18 00:05:24]

nit: beforeunload

(This is different from an unload handler, which runs later.)

[ananta, 2017/02/18 00:17:51]

Done. Thanks

+  // is_history_navigation_in_new_child is true. This indicates a newly created
+  // child frame which does not have an unload handler.

[Charlie Reis, 2017/02/18 00:05:24]

nit: s/an unload handler/a beforeunload handler/

[ananta, 2017/02/18 00:17:52]

Done.

   bool should_dispatch_beforeunload =
       !is_same_document_history_load &&
-      frame_tree_node->current_frame_host()->ShouldDispatchBeforeUnload();
+      frame_tree_node->current_frame_host()->ShouldDispatchBeforeUnload() &&
+      !is_history_navigation_in_new_child;

[Charlie Reis, 2017/02/18 00:05:24]

minor nit: I might put this before the ShouldDispatchBeforeUnload line, just
because it seems slightly more related to the is_same_document_history_load
case.

[ananta, 2017/02/18 00:17:52]

Done.

   FrameMsg_Navigate_Type::Value navigation_type = GetNavigationType(
       frame_tree_node->current_url(),  // old_url
       dest_url,                        // new_url