PlzNavigate: Updated navigation cancel policy for renderer-initiated requests.

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 e4afbe62a6628ddf93e7e7c52ef6c120d1189dc8..2d84d1e013f473a41d571c82f4cb57f818508963 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -707,10 +707,21 @@ void NavigatorImpl::OnBeginNavigation(
       switches::kEnableBrowserSideNavigation));
   DCHECK(frame_tree_node);
 
-  // This is a renderer-initiated navigation, so generate a new
-  // NavigationRequest and store it in the map.
-  // TODO(clamy): Renderer-initiated navigations should not always cancel the
-  // current one.
+  NavigationRequest* ongoing_navigation_request =
+      navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
+
+  // The renderer navigation request is ignored iff a) there is an ongoing

[Charlie Reis, 2015/02/17 23:48:29]

nit: Please keep the term "renderer-initiated navigation" to be consistent with
other code.

[carlosk, 2015/02/18 15:28:25]

Done.

+  // 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) {
+    return;
+  }
+
+  // In all other cases the current navigation, if any, is canceled and a new
+  // NavigationRequest is created and stored in the map.

[Charlie Reis, 2015/02/17 23:48:29]

Can you mention where it gets canceled?

(I don't see cancelation mentioned in the comment of CreateRendererInitiated. 
Is there code somewhere that tells the network stack, etc, that the old
navigation should be stopped?)

[clamy, 2015/02/18 12:45:33]

This is done upon destruction of the old NavigationRequest. It owns a
NavigationURLLoader, and the destruction of the NavigationURLLoader triggers a
call to the cancelation logic of the network stack.

[carlosk, 2015/02/18 15:28:25]

I changed the comment to clarify that.

[Charlie Reis, 2015/02/19 00:58:00]

Acknowledged.

   scoped_ptr<NavigationRequest> navigation_request =
       NavigationRequest::CreateRendererInitiated(
           frame_tree_node, common_params, begin_params, body);