Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(289)

Unified Diff: content/browser/web_contents/web_contents_impl_unittest.cc

Issue 903043002: Skip BeforeUnload on cross-site navigations when there are no handlers Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed comments Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/frame_host/render_frame_host_manager_unittest.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/web_contents/web_contents_impl_unittest.cc
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index ab40bcd466f7431fb7b60094387f79825a8ff74e..7bab786386eddf52cb88c6226136ca105d20f4c1 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -489,6 +489,7 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) {
// 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());
@@ -528,6 +529,7 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) {
// 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());
@@ -731,6 +733,7 @@ TEST_F(WebContentsImplTest, NavigateFromSitelessUrl) {
// 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(
@@ -955,9 +958,9 @@ TEST_F(WebContentsImplTest, CrossSiteComparesAgainstCurrentPage) {
}
// 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.
@@ -965,42 +968,61 @@ TEST_F(WebContentsImplTest, CrossSiteUnloadHandlers) {
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);
+
+ // Now navigate to a new site. There are no beforeUnload handlers, so we
+ // should 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
@@ -1018,6 +1040,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationPreempted) {
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");
@@ -1057,6 +1080,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationBackPreempted) {
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");
@@ -1104,6 +1128,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationBackPreempted) {
EXPECT_EQ(url3, entry3->GetURL());
EXPECT_EQ(instance3,
NavigationEntryImpl::FromNavigationEntry(entry3)->site_instance());
+ google_rfh->SendBeforeUnloadHandlersPresent(true);
// Go back within the site.
controller().GoBack();
@@ -1155,6 +1180,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationNotPreemptedByFrame) {
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");
@@ -1185,6 +1211,7 @@ TEST_F(WebContentsImplTest, CrossSiteNotPreemptedDuringBeforeUnload) {
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.
@@ -1223,6 +1250,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationCanceled) {
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");
« no previous file with comments | « content/browser/frame_host/render_frame_host_manager_unittest.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698