Only cancel navigation on user gesture

content/browser/frame_host/render_frame_host_manager_unittest.cc

Index: content/browser/frame_host/render_frame_host_manager_unittest.cc
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index 11c3c8a27041ecb08bc3e54363a4f5ecc5b2f8ed..1c270934e0af745f71253a21a7acae52aa50b330 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -901,7 +901,7 @@ TEST_F(RenderFrameHostManagerTest, Navigate) {
   EXPECT_FALSE(manager->pending_frame_host());
 
   // Commit.
-  manager->DidNavigateFrame(host);
+  manager->DidNavigateFrame(host, true);
   // Commit to SiteInstance should be delayed until RenderView commit.
   EXPECT_TRUE(host == manager->current_frame_host());
   ASSERT_TRUE(host);
@@ -922,7 +922,7 @@ TEST_F(RenderFrameHostManagerTest, Navigate) {
   EXPECT_FALSE(manager->pending_frame_host());
 
   // Commit.
-  manager->DidNavigateFrame(host);
+  manager->DidNavigateFrame(host, true);
   EXPECT_TRUE(host == manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
@@ -943,7 +943,7 @@ TEST_F(RenderFrameHostManagerTest, Navigate) {
   notifications.Reset();
 
   // Commit.
-  manager->DidNavigateFrame(manager->pending_frame_host());
+  manager->DidNavigateFrame(manager->pending_frame_host(), true);
   EXPECT_TRUE(host == manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
@@ -993,7 +993,7 @@ TEST_F(RenderFrameHostManagerTest, WebUI) {
   EXPECT_TRUE(manager->web_ui());
 
   // Commit.
-  manager->DidNavigateFrame(host);
+  manager->DidNavigateFrame(host, true);
   EXPECT_TRUE(
       host->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 }
@@ -1031,7 +1031,7 @@ TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) {
       host1->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
   // Commit and ensure we still have bindings.
-  manager1->DidNavigateFrame(host1);
+  manager1->DidNavigateFrame(host1, true);
   SiteInstance* webui_instance = host1->GetSiteInstance();
   EXPECT_EQ(host1, manager1->current_frame_host());
   EXPECT_TRUE(
@@ -1060,7 +1060,7 @@ TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) {
   EXPECT_TRUE(
       host2->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
-  manager2->DidNavigateFrame(host2);
+  manager2->DidNavigateFrame(host2, true);
 }
 
 // Tests that we don't end up in an inconsistent state if a page does a back and
@@ -1100,8 +1100,28 @@ TEST_F(RenderFrameHostManagerTest, PageDoesBackAndReload) {
   contents()->GetFrameTree()->root()->navigator()->DidNavigate(evil_rfh,
                                                                params);
 
-  // That should have cancelled the pending RFH, and the evil RFH should be the
-  // current one.
+  // That should NOT have cancelled the pending RFH.

[Charlie Reis, 2014/12/05 19:37:42]

Please add: ", because the reload did not have a user gesture.  Thus, the
pending back navigation will still eventually commit."

+  EXPECT_TRUE(contents()->GetRenderManagerForTesting()->
+    pending_render_view_host() != NULL);

[Charlie Reis, 2014/12/05 19:37:43]

Style nit: 4 space indent on an unfinished line.

git cl format can help catch these things.

+  EXPECT_TRUE(contents()->GetRenderManagerForTesting()->pending_frame_host() !=
+              NULL);
+  EXPECT_EQ(evil_rfh,
+            contents()->GetRenderManagerForTesting()->current_frame_host());
+  EXPECT_EQ(evil_rfh->GetRenderViewHost(),
+            contents()->GetRenderManagerForTesting()->current_host());
+
+  // Also we should not have a pending navigation entry.
+  EXPECT_TRUE(contents()->GetController().GetPendingEntry() == NULL);
+  NavigationEntry* entry = contents()->GetController().GetVisibleEntry();
+  ASSERT_TRUE(entry != NULL);
+  EXPECT_EQ(kUrl2, entry->GetURL());
+
+  // Now do the same but as a user gesture.
+  params.gesture = NavigationGestureUser;
+  contents()->GetFrameTree()->root()->navigator()->DidNavigate(evil_rfh,
+                                                               params);
+
+  // User navigation should have cancelled the pending RFH.
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->
       pending_render_view_host() == NULL);
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->pending_frame_host() ==
@@ -1113,7 +1133,7 @@ TEST_F(RenderFrameHostManagerTest, PageDoesBackAndReload) {
 
   // Also we should not have a pending navigation entry.
   EXPECT_TRUE(contents()->GetController().GetPendingEntry() == NULL);
-  NavigationEntry* entry = contents()->GetController().GetVisibleEntry();
+  entry = contents()->GetController().GetVisibleEntry();
   ASSERT_TRUE(entry != NULL);
   EXPECT_EQ(kUrl2, entry->GetURL());
 }
@@ -1500,7 +1520,7 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) {
   EXPECT_EQ(manager->current_frame_host()->GetSiteInstance(), instance);
 
   // Commit.
-  manager->DidNavigateFrame(host);
+  manager->DidNavigateFrame(host, true);
   // Commit to SiteInstance should be delayed until RenderView commit.
   EXPECT_EQ(host, manager->current_frame_host());
   ASSERT_TRUE(host);
@@ -1521,7 +1541,7 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) {
   EXPECT_FALSE(manager->pending_frame_host());
 
   // Commit.
-  manager->DidNavigateFrame(host);
+  manager->DidNavigateFrame(host, true);
   EXPECT_EQ(host, manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_EQ(host->GetSiteInstance(), instance);
@@ -1561,7 +1581,7 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) {
   notifications.Reset();
 
   // Commit.
-  manager->DidNavigateFrame(host);
+  manager->DidNavigateFrame(host, true);
 
   // Commit to SiteInstance should be delayed until RenderFrame commits.
   EXPECT_EQ(host, manager->current_frame_host());