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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/lifetime/browser_close_manager_browsertest.cc

Issue 2801813005: Only show a beforeunload dialog if a frame has had a user gesture since its load. (Closed)
Patch Set: aw Created 3 years, 8 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 | « chrome/browser/devtools/devtools_sanity_browsertest.cc ('k') | chrome/browser/ui/browser_browsertest.cc » ('j') | chrome/browser/ui/browser_browsertest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/lifetime/browser_close_manager_browsertest.cc
diff --git a/chrome/browser/lifetime/browser_close_manager_browsertest.cc b/chrome/browser/lifetime/browser_close_manager_browsertest.cc
index 63bbc8654d0139a4b20b1e6c75ad46da77a869f2..6bb179325ed4ab1aae2fb260ff093e4d49e51901 100644
--- a/chrome/browser/lifetime/browser_close_manager_browsertest.cc
+++ b/chrome/browser/lifetime/browser_close_manager_browsertest.cc
@@ -71,13 +71,13 @@ app_modal::NativeAppModalDialog* GetNextDialog() {
return js_dialog->native_dialog();
}
-// Note: call |DisableHangMonitor| on the relevant WebContents or Browser before
+// Note: call |PrepareForDialog| on the relevant WebContents or Browser before
// trying to close it, to avoid flakiness. https://crbug.com/519646
void AcceptClose() {
GetNextDialog()->AcceptAppModalDialog();
}
-// Note: call |DisableHangMonitor| on the relevant WebContents or Browser before
+// Note: call |PrepareForDialog| on the relevant WebContents or Browser before
// trying to close it, to avoid flakiness. https://crbug.com/519646
void CancelClose() {
GetNextDialog()->CancelAppModalDialog();
@@ -290,17 +290,22 @@ class BrowserCloseManagerBrowserTest
observer.NumDownloadsSeenInState(content::DownloadItem::IN_PROGRESS));
}
- // Makes sure that the beforeunload hang monitor will not trigger. That must
- // be called before close action when using |AcceptClose| or |CancelClose|, to
- // ensure the timeout does not prevent the dialog from appearing.
- // https://crbug.com/519646
- void DisableHangMonitor(content::WebContents* web_contents) {
+ // Prepares a WebContents or Browser for a beforeunload dialog.
+ //
+ // This comprises two things. First, it makes sure that the beforeunload hang
+ // monitor will not trigger, otherwise timeout might prevent the dialog from
+ // appearing. See https://crbug.com/519646 .
+ //
+ // Second, beforeunload dialogs require a user gesture, so it makes one.
+ void PrepareForDialog(content::WebContents* web_contents) {
Charlie Reis 2017/04/10 06:20:11 This might be a good candidate for a utility in co
Avi (use Gerrit) 2017/05/05 15:30:04 Done.
web_contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting();
+ for (auto* frame : web_contents->GetAllFrames())
+ frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
}
- void DisableHangMonitor(Browser* browser) {
+ void PrepareForDialog(Browser* browser) {
for (int i = 0; i < browser->tab_strip_model()->count(); i++)
- DisableHangMonitor(browser->tab_strip_model()->GetWebContentsAt(i));
+ PrepareForDialog(browser->tab_strip_model()->GetWebContentsAt(i));
}
std::vector<Browser*> browsers_;
@@ -310,7 +315,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest, TestSingleTabShutdown) {
ASSERT_TRUE(embedded_test_server()->Start());
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browser());
+ PrepareForDialog(browser());
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
@@ -334,7 +339,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
ASSERT_TRUE(embedded_test_server()->Start());
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browser());
+ PrepareForDialog(browser());
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
@@ -363,7 +368,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
AddBlankTabAndShow(browser());
ASSERT_NO_FATAL_FAILURE(
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAboutURL)));
- DisableHangMonitor(browser());
+ PrepareForDialog(browser());
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
@@ -411,8 +416,8 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest, TestMultipleWindows) {
browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[0]);
+ PrepareForDialog(browsers_[1]);
// Cancel shutdown on the first beforeunload event.
{
@@ -466,7 +471,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_[0], embedded_test_server()->GetURL("/beforeunload_hang.html")));
// Disable the hang monitor in the tab that is not expected to hang, so that
// the dialog is guaranteed to show.
- DisableHangMonitor(browsers_[0]->tab_strip_model()->GetWebContentsAt(1));
+ PrepareForDialog(browsers_[0]->tab_strip_model()->GetWebContentsAt(1));
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
@@ -502,7 +507,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_[2], embedded_test_server()->GetURL("/beforeunload_hang.html")));
// Disable the hang monitor in the tab that is not expected to hang, so that
// the dialog is guaranteed to show.
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[1]);
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
@@ -547,7 +552,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
}
// Disable the hang monitor in the tab that is not expected to hang, so that
// the dialog is guaranteed to show.
- DisableHangMonitor(
+ PrepareForDialog(
browsers_[0]->tab_strip_model()->GetWebContentsAt(kResposiveTabIndex));
RepeatedNotificationObserver cancel_observer(
@@ -596,7 +601,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
}
// Disable the hang monitor in the tab that is not expected to hang, so that
// the dialog is guaranteed to show.
- DisableHangMonitor(browsers_[kResposiveBrowserIndex]);
+ PrepareForDialog(browsers_[kResposiveBrowserIndex]);
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, kResposiveBrowserIndex + 1);
@@ -625,7 +630,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
ASSERT_TRUE(embedded_test_server()->Start());
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
+ PrepareForDialog(browsers_[0]);
RepeatedNotificationObserver close_observer(
chrome::NOTIFICATION_BROWSER_CLOSED, 2);
@@ -644,7 +649,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
ASSERT_TRUE(embedded_test_server()->Start());
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
+ PrepareForDialog(browsers_[0]);
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
@@ -652,7 +657,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_.push_back(CreateBrowser(browser()->profile()));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[1]);
ASSERT_NO_FATAL_FAILURE(AcceptClose());
ASSERT_NO_FATAL_FAILURE(CancelClose());
cancel_observer.Wait();
@@ -680,8 +685,8 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[0]);
+ PrepareForDialog(browsers_[1]);
RepeatedNotificationObserver close_observer(
chrome::NOTIFICATION_BROWSER_CLOSED, 2);
@@ -706,8 +711,8 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[0]);
+ PrepareForDialog(browsers_[1]);
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
@@ -719,8 +724,8 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
AddBlankTabAndShow(browsers_[1]);
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[0]);
+ PrepareForDialog(browsers_[1]);
ASSERT_NO_FATAL_FAILURE(AcceptClose());
ASSERT_NO_FATAL_FAILURE(CancelClose());
cancel_observer.Wait();
@@ -779,7 +784,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
->NeedToFireBeforeUnload());
EXPECT_EQ(2, browser2->tab_strip_model()->count());
- DisableHangMonitor(browser2);
+ PrepareForDialog(browser2);
// The test.
@@ -843,7 +848,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
ASSERT_TRUE(embedded_test_server()->Start());
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
+ PrepareForDialog(browsers_[0]);
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
@@ -852,7 +857,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_.push_back(CreateBrowser(browser()->profile()));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[1]);
browsers_[1]->tab_strip_model()->CloseAllTabs();
ASSERT_NO_FATAL_FAILURE(CancelClose());
ASSERT_NO_FATAL_FAILURE(CancelClose());
@@ -878,7 +883,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
ASSERT_TRUE(embedded_test_server()->Start());
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
+ PrepareForDialog(browsers_[0]);
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
@@ -887,7 +892,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_.push_back(CreateBrowser(browser()->profile()));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[1]);
ASSERT_FALSE(browsers_[1]->ShouldCloseWindow());
ASSERT_NO_FATAL_FAILURE(CancelClose());
ASSERT_NO_FATAL_FAILURE(CancelClose());
@@ -916,8 +921,8 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
browsers_.push_back(CreateBrowser(browser()->profile()));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browsers_[1], embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browsers_[0]);
- DisableHangMonitor(browsers_[1]);
+ PrepareForDialog(browsers_[0]);
+ PrepareForDialog(browsers_[1]);
RepeatedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
@@ -1162,7 +1167,7 @@ IN_PROC_BROWSER_TEST_P(BrowserCloseManagerWithDownloadsBrowserTest,
ASSERT_NO_FATAL_FAILURE(CreateStalledDownload(browser()));
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL("/beforeunload.html")));
- DisableHangMonitor(browser());
+ PrepareForDialog(browser());
content::WindowedNotificationObserver cancel_observer(
chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED,
« no previous file with comments | « chrome/browser/devtools/devtools_sanity_browsertest.cc ('k') | chrome/browser/ui/browser_browsertest.cc » ('j') | chrome/browser/ui/browser_browsertest.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698