Skip BeforeUnload on cross-site navigations when there are no handlers

content/browser/web_contents/web_contents_impl_unittest.cc

 // Copyright (c) 2012 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/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
@@ skipping 471 matching lines @@
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
   controller().LoadURL(
       url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   orig_rfh->PrepareForCommit(url);
   contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED);
 
   // Keep the number of active frames in orig_rfh's SiteInstance non-zero so
   // that orig_rfh doesn't get deleted when it gets swapped out.
   orig_rfh->GetSiteInstance()->increment_active_frame_count();
+  orig_rfh->SendBeforeUnloadHandlersPresent(true);
 
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(orig_rfh->GetRenderViewHost(), contents()->GetRenderViewHost());
   EXPECT_EQ(url, contents()->GetLastCommittedURL());
   EXPECT_EQ(url, contents()->GetVisibleURL());
 
   // Navigate to new site
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
@@ skipping 19 matching lines @@
 
   // DidNavigate from the pending page
   contents()->TestDidNavigate(
       pending_rfh, 1, url2, ui::PAGE_TRANSITION_TYPED);
   SiteInstance* instance2 = contents()->GetSiteInstance();
 
   // Keep the number of active frames in pending_rfh's SiteInstance
   // non-zero so that orig_rfh doesn't get deleted when it gets
   // swapped out.
   pending_rfh->GetSiteInstance()->increment_active_frame_count();
+  pending_rfh->SendBeforeUnloadHandlersPresent(true);
 
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(pending_rfh, contents()->GetMainFrame());
   EXPECT_EQ(url2, contents()->GetLastCommittedURL());
   EXPECT_EQ(url2, contents()->GetVisibleURL());
   EXPECT_NE(instance1, instance2);
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
   // We keep the original RFH around, swapped out.
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->IsOnSwappedOutList(
       orig_rfh));
@@ skipping 183 matching lines @@
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(native_url, contents()->GetLastCommittedURL());
   EXPECT_EQ(url, contents()->GetVisibleURL());
   EXPECT_FALSE(contents()->GetPendingMainFrame());
   contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED);
 
   // Keep the number of active frames in orig_rfh's SiteInstance
   // non-zero so that orig_rfh doesn't get deleted when it gets
   // swapped out.
   orig_rfh->GetSiteInstance()->increment_active_frame_count();
+  orig_rfh->SendBeforeUnloadHandlersPresent(true);
 
   EXPECT_EQ(orig_instance, contents()->GetSiteInstance());
   EXPECT_TRUE(
       contents()->GetSiteInstance()->GetSiteURL().DomainIs("google.com"));
   EXPECT_EQ(url, contents()->GetLastCommittedURL());
 
   // Navigate to another new site (should create a new site instance).
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
@@ skipping 204 matching lines @@
       url3, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   EXPECT_FALSE(contents()->cross_navigation_pending());
   contents()->GetMainFrame()->PrepareForCommit(url3);
   contents()->TestDidNavigate(
       orig_rfh, 3, url3, ui::PAGE_TRANSITION_TYPED);
   SiteInstance* instance4 = contents()->GetSiteInstance();
   EXPECT_EQ(instance1, instance4);
 }
 
 // Test that the onbeforeunload and onunload handlers run when navigating
-// across site boundaries.
+// across site boundaries if needed.
 TEST_F(WebContentsImplTest, CrossSiteUnloadHandlers) {
-  TestRenderFrameHost* orig_rfh = contents()->GetMainFrame();
+  TestRenderFrameHost* rfh1 = contents()->GetMainFrame();
   SiteInstance* instance1 = contents()->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
   controller().LoadURL(
       url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   contents()->GetMainFrame()->PrepareForCommit(url);
-  contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED);
+  contents()->TestDidNavigate(rfh1, 1, url, ui::PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->cross_navigation_pending());
-  EXPECT_EQ(orig_rfh, contents()->GetMainFrame());
+  EXPECT_EQ(rfh1, contents()->GetMainFrame());
+  rfh1->SendBeforeUnloadHandlersPresent(true);
 
   // Navigate to new site, but simulate an onbeforeunload denial.
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
-  EXPECT_TRUE(orig_rfh->IsWaitingForBeforeUnloadACK());
+  EXPECT_TRUE(rfh1->IsWaitingForBeforeUnloadACK());
   base::TimeTicks now = base::TimeTicks::Now();
-  orig_rfh->OnMessageReceived(
+  rfh1->OnMessageReceived(
       FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
-  EXPECT_FALSE(orig_rfh->IsWaitingForBeforeUnloadACK());
+  EXPECT_FALSE(rfh1->IsWaitingForBeforeUnloadACK());
   EXPECT_FALSE(contents()->cross_navigation_pending());
-  EXPECT_EQ(orig_rfh, contents()->GetMainFrame());
+  EXPECT_EQ(rfh1, contents()->GetMainFrame());
 
   // Navigate again, but simulate an onbeforeunload approval.
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
-  EXPECT_TRUE(orig_rfh->IsWaitingForBeforeUnloadACK());
+  EXPECT_TRUE(rfh1->IsWaitingForBeforeUnloadACK());
   now = base::TimeTicks::Now();
-  orig_rfh->PrepareForCommit(url2);
-  EXPECT_FALSE(orig_rfh->IsWaitingForBeforeUnloadACK());
+  rfh1->PrepareForCommit(url2);
+  EXPECT_FALSE(rfh1->IsWaitingForBeforeUnloadACK());
   EXPECT_TRUE(contents()->cross_navigation_pending());
-  TestRenderFrameHost* pending_rfh = contents()->GetPendingMainFrame();
-
-  // We won't hear DidNavigate until the onunload handler has finished running.
+  TestRenderFrameHost* rfh2 = contents()->GetPendingMainFrame();
 
   // DidNavigate from the pending page.
   contents()->TestDidNavigate(
-      pending_rfh, 1, url2, ui::PAGE_TRANSITION_TYPED);
+      rfh2, 1, url2, ui::PAGE_TRANSITION_TYPED);
   SiteInstance* instance2 = contents()->GetSiteInstance();
   EXPECT_FALSE(contents()->cross_navigation_pending());
-  EXPECT_EQ(pending_rfh, contents()->GetMainFrame());
+  EXPECT_EQ(rfh2, contents()->GetMainFrame());
   EXPECT_NE(instance1, instance2);
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
+

[carlosk, 2015/02/17 12:52:09]

Suggestion: split it here into a new test.

[clamy, 2015/02/18 13:27:23]

I think it makes sense for it to be in this test, which is testing beforeUnload
in general.

+  // Now navigate to a new site. There is no beforeUnload handlers, so we should

[Charlie Reis, 2015/02/17 23:04:31]

nit: There are

[clamy, 2015/02/18 13:27:23]

Done.

+  // not be waiting for a BeforeUnloadACK.
+  const GURL url3("http://www.chromium.org");
+  rfh2->GetSiteInstance()->increment_active_frame_count();
+  controller().LoadURL(
+      url3, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
+  EXPECT_FALSE(rfh2->IsWaitingForBeforeUnloadACK());
+  rfh2->PrepareForCommit(url3);
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  TestRenderFrameHost* rfh3 = contents()->GetPendingMainFrame();
+
+  // DidNavigate from the pending page.
+  contents()->TestDidNavigate(
+      rfh3, 1, url3, ui::PAGE_TRANSITION_TYPED);
+  SiteInstance* instance3 = contents()->GetSiteInstance();
+  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_EQ(rfh3, contents()->GetMainFrame());
+  EXPECT_NE(instance2, instance3);
+  EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
 }
 
 // Test that during a slow cross-site navigation, the original renderer can
 // navigate to a different URL and have it displayed, canceling the slow
 // navigation.
 TEST_F(WebContentsImplTest, CrossSiteNavigationPreempted) {
   TestRenderFrameHost* orig_rfh = contents()->GetMainFrame();
   SiteInstance* instance1 = contents()->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderFrameHost.
   const GURL url("http://www.google.com");
   controller().LoadURL(
       url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   contents()->GetMainFrame()->PrepareForCommit(url);
   contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(orig_rfh, contents()->GetMainFrame());
+  orig_rfh->SendBeforeUnloadHandlersPresent(true);
 
   // Navigate to new site, simulating an onbeforeunload approval.
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   EXPECT_TRUE(orig_rfh->IsWaitingForBeforeUnloadACK());
   orig_rfh->PrepareForCommit(url2);
   EXPECT_TRUE(contents()->cross_navigation_pending());
 
   // Suppose the original renderer navigates before the new one is ready.
@@ skipping 19 matching lines @@
   NavigationEntry* entry1 = controller().GetLastCommittedEntry();
   SiteInstance* instance1 = contents()->GetSiteInstance();
 
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(ntp_rfh, contents()->GetMainFrame());
   EXPECT_EQ(url1, entry1->GetURL());
   EXPECT_EQ(instance1,
             NavigationEntryImpl::FromNavigationEntry(entry1)->site_instance());
   EXPECT_TRUE(ntp_rfh->GetRenderViewHost()->GetEnabledBindings() &
               BINDINGS_POLICY_WEB_UI);
+  ntp_rfh->SendBeforeUnloadHandlersPresent(true);
 
   // Navigate to new site.
   const GURL url2("http://www.google.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   EXPECT_TRUE(contents()->cross_navigation_pending());
   TestRenderFrameHost* google_rfh = contents()->GetPendingMainFrame();
 
   // Simulate beforeunload approval.
   EXPECT_TRUE(ntp_rfh->IsWaitingForBeforeUnloadACK());
@@ skipping 27 matching lines @@
   NavigationEntry* entry3 = controller().GetLastCommittedEntry();
   SiteInstance* instance3 = contents()->GetSiteInstance();
 
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(google_rfh, contents()->GetMainFrame());
   EXPECT_EQ(instance2, instance3);
   EXPECT_FALSE(contents()->GetPendingMainFrame());
   EXPECT_EQ(url3, entry3->GetURL());
   EXPECT_EQ(instance3,
             NavigationEntryImpl::FromNavigationEntry(entry3)->site_instance());
+  google_rfh->SendBeforeUnloadHandlersPresent(true);
 
   // Go back within the site.
   controller().GoBack();
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(entry2, controller().GetPendingEntry());
 
   // Before that commits, go back again.
   controller().GoBack();
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_TRUE(contents()->GetPendingMainFrame());
@@ skipping 31 matching lines @@
   TestRenderFrameHost* orig_rfh = contents()->GetMainFrame();
 
   // Navigate to URL.  First URL should use the original RenderFrameHost.
   const GURL url("http://www.google.com");
   controller().LoadURL(
       url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   contents()->GetMainFrame()->PrepareForCommit(url);
   contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(orig_rfh, contents()->GetMainFrame());
+  orig_rfh->SendBeforeUnloadHandlersPresent(true);
 
   // Start navigating to new site.
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
 
   // Simulate a sub-frame navigation arriving and ensure the RVH is still
   // waiting for a before unload response.
   TestRenderFrameHost* child_rfh = orig_rfh->AppendChild("subframe");
   child_rfh->SendNavigateWithTransition(
@@ skipping 10 matching lines @@
 // Test that a cross-site navigation is not preempted if the previous
 // renderer sends a FrameNavigate message just before being told to stop.
 // We should only preempt the cross-site navigation if the previous renderer
 // has started a new navigation.  See http://crbug.com/79176.
 TEST_F(WebContentsImplTest, CrossSiteNotPreemptedDuringBeforeUnload) {
   // Navigate to NTP URL.
   const GURL url("chrome://blah");
   controller().LoadURL(
       url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   TestRenderFrameHost* orig_rfh = contents()->GetMainFrame();
+  orig_rfh->SendBeforeUnloadHandlersPresent(true);
   EXPECT_FALSE(contents()->cross_navigation_pending());
 
   // Navigate to new site, with the beforeunload request in flight.
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   TestRenderFrameHost* pending_rfh = contents()->GetPendingMainFrame();
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_TRUE(orig_rfh->IsWaitingForBeforeUnloadACK());
 
@@ skipping 18 matching lines @@
   SiteInstance* instance1 = contents()->GetSiteInstance();
 
   // Navigate to URL.  First URL should use original RenderFrameHost.
   const GURL url("http://www.google.com");
   controller().LoadURL(
       url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   contents()->GetMainFrame()->PrepareForCommit(url);
   contents()->TestDidNavigate(orig_rfh, 1, url, ui::PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(orig_rfh, contents()->GetMainFrame());
+  orig_rfh->SendBeforeUnloadHandlersPresent(true);
 
   // Navigate to new site, simulating an onbeforeunload approval.
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   EXPECT_TRUE(orig_rfh->IsWaitingForBeforeUnloadACK());
   contents()->GetMainFrame()->PrepareForCommit(url2);
   EXPECT_TRUE(contents()->cross_navigation_pending());
 
   // Simulate swap out message when the response arrives.
@@ skipping 1802 matching lines @@
   frame->SendBeforeUnloadHandlersPresent(false);
   EXPECT_FALSE(frame->SuddenTerminationAllowed());
   frame->SendBeforeUnloadHandlersPresent(true);
   frame->SendUnloadHandlersPresent(false);
   EXPECT_FALSE(frame->SuddenTerminationAllowed());
   frame->SendBeforeUnloadHandlersPresent(false);
   EXPECT_TRUE(frame->SuddenTerminationAllowed());
 }
 
 }  // namespace content