Only cancel navigation on user gesture

content/browser/frame_host/render_frame_host_manager_unittest.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 "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/histogram_tester.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
@@ skipping 883 matching lines @@
       NULL /* instance */, -1 /* page_id */, kUrl1, Referrer(),
       base::string16() /* title */, ui::PAGE_TRANSITION_TYPED,
       false /* is_renderer_init */);
   host = manager->Navigate(entry1);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_TRUE(host == manager->current_frame_host());
   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);
   EXPECT_FALSE(host->GetSiteInstance()->HasSite());
   host->GetSiteInstance()->SetSite(kUrl1);
 
   // 2) Navigate to next site. -------------------------
   const GURL kUrl2("http://www.google.com/foo");
   NavigationEntryImpl entry2(
       NULL /* instance */, -1 /* page_id */, kUrl2,
       Referrer(kUrl1, blink::WebReferrerPolicyDefault),
       base::string16() /* title */, ui::PAGE_TRANSITION_LINK,
       true /* is_renderer_init */);
   host = manager->Navigate(entry2);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_TRUE(host == manager->current_frame_host());
   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());
 
   // 3) Cross-site navigate to next site. --------------
   const GURL kUrl3("http://webkit.org/");
   NavigationEntryImpl entry3(
       NULL /* instance */, -1 /* page_id */, kUrl3,
       Referrer(kUrl2, blink::WebReferrerPolicyDefault),
       base::string16() /* title */, ui::PAGE_TRANSITION_LINK,
       false /* is_renderer_init */);
   host = manager->Navigate(entry3);
 
   // A new RenderFrameHost should be created.
   EXPECT_TRUE(manager->pending_frame_host());
   ASSERT_EQ(host, manager->pending_frame_host());
 
   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());
   // Check the pending RenderFrameHost has been committed.
   EXPECT_FALSE(manager->pending_frame_host());
 
   // We should observe a notification.
   EXPECT_TRUE(
       notifications.Check1AndReset(NOTIFICATION_RENDER_VIEW_HOST_CHANGED));
 }
@@ skipping 29 matching lines @@
   // get loaded in between.
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
   EXPECT_EQ(kUrl, host->GetSiteInstance()->GetSiteURL());
 
   // The Web UI is committed immediately because the RenderViewHost has not been
   // used yet. UpdateStateForNavigate() took the short cut path.
   EXPECT_FALSE(manager->pending_web_ui());
   EXPECT_TRUE(manager->web_ui());
 
   // Commit.
-  manager->DidNavigateFrame(host);
+  manager->DidNavigateFrame(host, true);
   EXPECT_TRUE(
       host->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 }
 
 // Tests that we can open a WebUI link in a new tab from a WebUI page and still
 // grant the correct bindings.  http://crbug.com/189101.
 TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) {
   set_should_create_webui(true);
   SiteInstance* blank_instance = SiteInstance::Create(browser_context());
 
@@ skipping 17 matching lines @@
   RenderFrameHostImpl* host1 = manager1->Navigate(entry1);
 
   // We should have a pending navigation to the WebUI RenderViewHost.
   // It should already have bindings.
   EXPECT_EQ(host1, manager1->pending_frame_host());
   EXPECT_NE(host1, manager1->current_frame_host());
   EXPECT_TRUE(
       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(
       host1->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
   // Now simulate clicking a link that opens in a new tab.
   scoped_ptr<TestWebContents> web_contents2(
       TestWebContents::Create(browser_context(), webui_instance));
   RenderFrameHostManager* manager2 =
       web_contents2->GetRenderManagerForTesting();
   // Make sure the new RVH is considered live.  This is usually done in
   // RenderWidgetHost::Init when opening a new tab from a link.
   manager2->current_host()->CreateRenderView(
       base::string16(), -1, MSG_ROUTING_NONE, -1, false);
 
   const GURL kUrl2("chrome://foo/bar");
   NavigationEntryImpl entry2(NULL /* instance */, -1 /* page_id */, kUrl2,
                              Referrer(), base::string16() /* title */,
                              ui::PAGE_TRANSITION_LINK,
                              true /* is_renderer_init */);
   RenderFrameHostImpl* host2 = manager2->Navigate(entry2);
 
   // No cross-process transition happens because we are already in the right
   // SiteInstance.  We should grant bindings immediately.
   EXPECT_EQ(host2, manager2->current_frame_host());
   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
 // then reload. http://crbug.com/51680

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

Please add:
Also tests that only user-gesture navigations can interrupt cross-process
navigations.  http://crbug.com/75195

 TEST_F(RenderFrameHostManagerTest, PageDoesBackAndReload) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.evil-site.com/");
 
   // Navigate to a safe site, then an evil site.
   // This will switch RenderFrameHosts.  We cannot assert that the first and
   // second RFHs are different, though, because the first one may be promptly
   // deleted.
   contents()->NavigateAndCommit(kUrl1);
   contents()->NavigateAndCommit(kUrl2);
@@ skipping 15 matching lines @@
   params.transition = ui::PAGE_TRANSITION_CLIENT_REDIRECT;
   params.should_update_history = false;
   params.gesture = NavigationGestureAuto;
   params.was_within_same_page = false;
   params.is_post = false;
   params.page_state = PageState::CreateFromURL(kUrl2);
 
   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() ==
               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();
+  entry = contents()->GetController().GetVisibleEntry();
   ASSERT_TRUE(entry != NULL);
   EXPECT_EQ(kUrl2, entry->GetURL());
 }
 
 // Ensure that we can go back and forward even if a SwapOut ACK isn't received.
 // See http://crbug.com/93427.
 TEST_F(RenderFrameHostManagerTest, NavigateAfterMissingSwapOutACK) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
 
@@ skipping 366 matching lines @@
       base::string16() /* title */, ui::PAGE_TRANSITION_TYPED,
       false /* is_renderer_init */);
   host = manager->Navigate(entry1);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_TRUE(host == manager->current_frame_host());
   EXPECT_FALSE(manager->pending_frame_host());
   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);
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
 
   // 2) Navigate to a different domain. -------------------------
   // Guests stay in the same process on navigation.
   const GURL kUrl2("http://www.chromium.org");
   NavigationEntryImpl entry2(
       NULL /* instance */, -1 /* page_id */, kUrl2,
       Referrer(kUrl1, blink::WebReferrerPolicyDefault),
       base::string16() /* title */, ui::PAGE_TRANSITION_LINK,
       true /* is_renderer_init */);
   host = manager->Navigate(entry2);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_EQ(host, manager->current_frame_host());
   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);
 }
 
 // Test that we cancel a pending RVH if we close the tab while it's pending.
 // http://crbug.com/294697.
 TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) {
   TestNotificationTracker notifications;
 
@@ skipping 19 matching lines @@
   // The RenderFrameHost created in Init will be reused.
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(manager->pending_frame_host());
 
   // We should observe a notification.
   EXPECT_TRUE(
       notifications.Check1AndReset(NOTIFICATION_RENDER_VIEW_HOST_CHANGED));
   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());
   EXPECT_FALSE(host->GetSiteInstance()->HasSite());
   host->GetSiteInstance()->SetSite(kUrl1);
 
   // 2) Cross-site navigate to next site. -------------------------
   const GURL kUrl2("http://www.example.com");
   NavigationEntryImpl entry2(
       NULL /* instance */, -1 /* page_id */, kUrl2, Referrer(),
@@ skipping 223 matching lines @@
    pending_rfh->GetSiteInstance()->increment_active_frame_count();
 
     contents()->GetMainFrame()->OnMessageReceived(
         FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
     EXPECT_FALSE(contents()->cross_navigation_pending());
     EXPECT_FALSE(rfh_deleted_observer.deleted());
   }
 }
 
 }  // namespace content