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

Side by Side Diff: chrome/browser/ui/browser_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 unified diff | Download patch
OLDNEW
1 // Copyright 2012 The Chromium Authors. All rights reserved. 1 // Copyright 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/ui/browser.h" 5 #include "chrome/browser/ui/browser.h"
6 6
7 #include <stddef.h> 7 #include <stddef.h>
8 #include <stdint.h> 8 #include <stdint.h>
9 9
10 #include <memory> 10 #include <memory>
(...skipping 606 matching lines...) Expand 10 before | Expand all | Expand 10 after
617 IN_PROC_BROWSER_TEST_F(BrowserTest, SadTabCancelsDialogs) { 617 IN_PROC_BROWSER_TEST_F(BrowserTest, SadTabCancelsDialogs) {
618 ASSERT_TRUE(embedded_test_server()->Start()); 618 ASSERT_TRUE(embedded_test_server()->Start());
619 host_resolver()->AddRule("www.example.com", "127.0.0.1"); 619 host_resolver()->AddRule("www.example.com", "127.0.0.1");
620 GURL beforeunload_url(embedded_test_server()->GetURL("/beforeunload.html")); 620 GURL beforeunload_url(embedded_test_server()->GetURL("/beforeunload.html"));
621 ui_test_utils::NavigateToURL(browser(), beforeunload_url); 621 ui_test_utils::NavigateToURL(browser(), beforeunload_url);
622 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); 622 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
623 // Disable the hang monitor, otherwise there will be a race between the 623 // Disable the hang monitor, otherwise there will be a race between the
624 // beforeunload dialog and the beforeunload hang timer. 624 // beforeunload dialog and the beforeunload hang timer.
625 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting(); 625 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting();
626 626
627 // JavaScript onbeforeunload dialogs require a user gesture.
Nico 2017/04/07 20:05:28 You're adding this exactly in the tests where you
Charlie Reis 2017/04/10 06:20:11 It does seem like the majority of cases would bene
Avi (use Gerrit) 2017/05/05 15:30:04 Done.
628 for (auto* frame :
629 browser()->tab_strip_model()->GetActiveWebContents()->GetAllFrames()) {
630 frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
631 }
632
627 // Start a navigation to trigger the beforeunload dialog. 633 // Start a navigation to trigger the beforeunload dialog.
628 contents->GetMainFrame()->ExecuteJavaScriptForTests( 634 contents->GetMainFrame()->ExecuteJavaScriptForTests(
629 ASCIIToUTF16("window.location.href = 'about:blank'")); 635 ASCIIToUTF16("window.location.href = 'about:blank'"));
630 AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog(); 636 AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog();
631 EXPECT_TRUE(alert->IsValid()); 637 EXPECT_TRUE(alert->IsValid());
632 AppModalDialogQueue* dialog_queue = AppModalDialogQueue::GetInstance(); 638 AppModalDialogQueue* dialog_queue = AppModalDialogQueue::GetInstance();
633 EXPECT_TRUE(dialog_queue->HasActiveDialog()); 639 EXPECT_TRUE(dialog_queue->HasActiveDialog());
634 640
635 // Crash the renderer process and ensure the dialog is gone. 641 // Crash the renderer process and ensure the dialog is gone.
636 content::RenderProcessHost* child_process = contents->GetRenderProcessHost(); 642 content::RenderProcessHost* child_process = contents->GetRenderProcessHost();
(...skipping 69 matching lines...) Expand 10 before | Expand all | Expand 10 after
706 // Test for crbug.com/22004. Reloading a page with a before unload handler and 712 // Test for crbug.com/22004. Reloading a page with a before unload handler and
707 // then canceling the dialog should not leave the throbber spinning. 713 // then canceling the dialog should not leave the throbber spinning.
708 IN_PROC_BROWSER_TEST_F(BrowserTest, ReloadThenCancelBeforeUnload) { 714 IN_PROC_BROWSER_TEST_F(BrowserTest, ReloadThenCancelBeforeUnload) {
709 GURL url(std::string("data:text/html,") + kBeforeUnloadHTML); 715 GURL url(std::string("data:text/html,") + kBeforeUnloadHTML);
710 ui_test_utils::NavigateToURL(browser(), url); 716 ui_test_utils::NavigateToURL(browser(), url);
711 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); 717 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
712 // Disable the hang monitor, otherwise there will be a race between the 718 // Disable the hang monitor, otherwise there will be a race between the
713 // beforeunload dialog and the beforeunload hang timer. 719 // beforeunload dialog and the beforeunload hang timer.
714 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting(); 720 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting();
715 721
722 // JavaScript onbeforeunload dialogs require a user gesture.
723 for (auto* frame :
724 browser()->tab_strip_model()->GetActiveWebContents()->GetAllFrames()) {
725 frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
726 }
727
716 // Navigate to another page, but click cancel in the dialog. Make sure that 728 // Navigate to another page, but click cancel in the dialog. Make sure that
717 // the throbber stops spinning. 729 // the throbber stops spinning.
718 chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB); 730 chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB);
719 AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog(); 731 AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog();
720 732
721 FailedCommitWatcher watcher(contents); 733 FailedCommitWatcher watcher(contents);
722 alert->CloseModalDialog(); 734 alert->CloseModalDialog();
723 if (content::IsBrowserSideNavigationEnabled()) 735 if (content::IsBrowserSideNavigationEnabled())
724 watcher.Wait(); 736 watcher.Wait();
725 EXPECT_FALSE(contents->IsLoading()); 737 EXPECT_FALSE(contents->IsLoading());
(...skipping 104 matching lines...) Expand 10 before | Expand all | Expand 10 after
830 SetBrowserClientForTesting(&new_client); 842 SetBrowserClientForTesting(&new_client);
831 843
832 // Navigate to a page with a beforeunload handler. 844 // Navigate to a page with a beforeunload handler.
833 GURL url(embedded_test_server()->GetURL("/beforeunload.html")); 845 GURL url(embedded_test_server()->GetURL("/beforeunload.html"));
834 ui_test_utils::NavigateToURL(browser(), url); 846 ui_test_utils::NavigateToURL(browser(), url);
835 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); 847 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
836 // Disable the hang monitor, otherwise there will be a race between the 848 // Disable the hang monitor, otherwise there will be a race between the
837 // beforeunload dialog and the beforeunload hang timer. 849 // beforeunload dialog and the beforeunload hang timer.
838 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting(); 850 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting();
839 851
852 // JavaScript onbeforeunload dialogs require a user gesture.
853 for (auto* frame :
854 browser()->tab_strip_model()->GetActiveWebContents()->GetAllFrames()) {
855 frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
856 }
857
840 // Navigate to a URL that redirects to another process and approve the 858 // Navigate to a URL that redirects to another process and approve the
841 // beforeunload dialog that pops up. 859 // beforeunload dialog that pops up.
842 content::WindowedNotificationObserver nav_observer( 860 content::WindowedNotificationObserver nav_observer(
843 content::NOTIFICATION_NAV_ENTRY_COMMITTED, 861 content::NOTIFICATION_NAV_ENTRY_COMMITTED,
844 content::NotificationService::AllSources()); 862 content::NotificationService::AllSources());
845 GURL https_url(https_test_server.GetURL("/title1.html")); 863 GURL https_url(https_test_server.GetURL("/title1.html"));
846 GURL redirect_url( 864 GURL redirect_url(
847 embedded_test_server()->GetURL("/server-redirect?" + https_url.spec())); 865 embedded_test_server()->GetURL("/server-redirect?" + https_url.spec()));
848 browser()->OpenURL(OpenURLParams(redirect_url, Referrer(), 866 browser()->OpenURL(OpenURLParams(redirect_url, Referrer(),
849 WindowOpenDisposition::CURRENT_TAB, 867 WindowOpenDisposition::CURRENT_TAB,
(...skipping 12 matching lines...) Expand all
862 // the URL to the previous page's URL. 880 // the URL to the previous page's URL.
863 IN_PROC_BROWSER_TEST_F(BrowserTest, CancelBeforeUnloadResetsURL) { 881 IN_PROC_BROWSER_TEST_F(BrowserTest, CancelBeforeUnloadResetsURL) {
864 GURL url(ui_test_utils::GetTestUrl(base::FilePath( 882 GURL url(ui_test_utils::GetTestUrl(base::FilePath(
865 base::FilePath::kCurrentDirectory), base::FilePath(kBeforeUnloadFile))); 883 base::FilePath::kCurrentDirectory), base::FilePath(kBeforeUnloadFile)));
866 ui_test_utils::NavigateToURL(browser(), url); 884 ui_test_utils::NavigateToURL(browser(), url);
867 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); 885 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
868 // Disable the hang monitor, otherwise there will be a race between the 886 // Disable the hang monitor, otherwise there will be a race between the
869 // beforeunload dialog and the beforeunload hang timer. 887 // beforeunload dialog and the beforeunload hang timer.
870 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting(); 888 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting();
871 889
890 // JavaScript onbeforeunload dialogs require a user gesture.
891 for (auto* frame :
892 browser()->tab_strip_model()->GetActiveWebContents()->GetAllFrames()) {
893 frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
894 }
895
872 // Navigate to a page that triggers a cross-site transition. 896 // Navigate to a page that triggers a cross-site transition.
873 ASSERT_TRUE(embedded_test_server()->Start()); 897 ASSERT_TRUE(embedded_test_server()->Start());
874 GURL url2(embedded_test_server()->GetURL("/title1.html")); 898 GURL url2(embedded_test_server()->GetURL("/title1.html"));
875 browser()->OpenURL(OpenURLParams(url2, Referrer(), 899 browser()->OpenURL(OpenURLParams(url2, Referrer(),
876 WindowOpenDisposition::CURRENT_TAB, 900 WindowOpenDisposition::CURRENT_TAB,
877 ui::PAGE_TRANSITION_TYPED, false)); 901 ui::PAGE_TRANSITION_TYPED, false));
878 902
879 content::WindowedNotificationObserver host_destroyed_observer( 903 content::WindowedNotificationObserver host_destroyed_observer(
880 content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED, 904 content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
881 content::NotificationService::AllSources()); 905 content::NotificationService::AllSources());
(...skipping 54 matching lines...) Expand 10 before | Expand all | Expand 10 after
936 // Test that when a page has an onbeforeunload handler, reloading a page shows a 960 // Test that when a page has an onbeforeunload handler, reloading a page shows a
937 // different dialog than navigating to a different page. 961 // different dialog than navigating to a different page.
938 IN_PROC_BROWSER_TEST_F(BrowserTest, BeforeUnloadVsBeforeReload) { 962 IN_PROC_BROWSER_TEST_F(BrowserTest, BeforeUnloadVsBeforeReload) {
939 GURL url(std::string("data:text/html,") + kBeforeUnloadHTML); 963 GURL url(std::string("data:text/html,") + kBeforeUnloadHTML);
940 ui_test_utils::NavigateToURL(browser(), url); 964 ui_test_utils::NavigateToURL(browser(), url);
941 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); 965 WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
942 // Disable the hang monitor, otherwise there will be a race between the 966 // Disable the hang monitor, otherwise there will be a race between the
943 // beforeunload dialog and the beforeunload hang timer. 967 // beforeunload dialog and the beforeunload hang timer.
944 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting(); 968 contents->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting();
945 969
970 // JavaScript onbeforeunload dialogs require a user gesture.
971 for (auto* frame :
972 browser()->tab_strip_model()->GetActiveWebContents()->GetAllFrames()) {
973 frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
974 }
975
946 // Reload the page, and check that we get a "before reload" dialog. 976 // Reload the page, and check that we get a "before reload" dialog.
947 chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB); 977 chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB);
948 AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog(); 978 AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog();
949 EXPECT_TRUE(static_cast<JavaScriptAppModalDialog*>(alert)->is_reload()); 979 EXPECT_TRUE(static_cast<JavaScriptAppModalDialog*>(alert)->is_reload());
950 980
951 // Cancel the reload. 981 // Cancel the reload.
952 FailedCommitWatcher watcher(contents); 982 FailedCommitWatcher watcher(contents);
953 alert->native_dialog()->CancelAppModalDialog(); 983 alert->native_dialog()->CancelAppModalDialog();
954 if (content::IsBrowserSideNavigationEnabled()) 984 if (content::IsBrowserSideNavigationEnabled())
955 watcher.Wait(); 985 watcher.Wait();
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
1006 } 1036 }
1007 }; 1037 };
1008 1038
1009 // Disabled, http://crbug.com/159214 . 1039 // Disabled, http://crbug.com/159214 .
1010 IN_PROC_BROWSER_TEST_F(BeforeUnloadAtQuitWithTwoWindows, 1040 IN_PROC_BROWSER_TEST_F(BeforeUnloadAtQuitWithTwoWindows,
1011 DISABLED_IfThisTestTimesOutItIndicatesFAILURE) { 1041 DISABLED_IfThisTestTimesOutItIndicatesFAILURE) {
1012 // In the first browser, set up a page that has a beforeunload handler. 1042 // In the first browser, set up a page that has a beforeunload handler.
1013 GURL url(std::string("data:text/html,") + kBeforeUnloadHTML); 1043 GURL url(std::string("data:text/html,") + kBeforeUnloadHTML);
1014 ui_test_utils::NavigateToURL(browser(), url); 1044 ui_test_utils::NavigateToURL(browser(), url);
1015 1045
1046 // JavaScript onbeforeunload dialogs require a user gesture.
1047 for (auto* frame :
1048 browser()->tab_strip_model()->GetActiveWebContents()->GetAllFrames()) {
1049 frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
1050 }
1051
1016 // Open a second browser window at about:blank. 1052 // Open a second browser window at about:blank.
1017 ui_test_utils::BrowserAddedObserver browser_added_observer; 1053 ui_test_utils::BrowserAddedObserver browser_added_observer;
1018 chrome::NewEmptyWindow(browser()->profile()); 1054 chrome::NewEmptyWindow(browser()->profile());
1019 Browser* second_window = browser_added_observer.WaitForSingleNewBrowser(); 1055 Browser* second_window = browser_added_observer.WaitForSingleNewBrowser();
1020 ui_test_utils::NavigateToURL(second_window, GURL(url::kAboutBlankURL)); 1056 ui_test_utils::NavigateToURL(second_window, GURL(url::kAboutBlankURL));
1021 1057
1022 // Tell the application to quit. IDC_EXIT calls AttemptUserExit, which on 1058 // Tell the application to quit. IDC_EXIT calls AttemptUserExit, which on
1023 // everything but ChromeOS allows unload handlers to block exit. On that 1059 // everything but ChromeOS allows unload handlers to block exit. On that
1024 // platform, though, it exits unconditionally. See the comment and bug ID 1060 // platform, though, it exits unconditionally. See the comment and bug ID
1025 // in AttemptUserExit() in application_lifetime.cc. 1061 // in AttemptUserExit() in application_lifetime.cc.
(...skipping 1848 matching lines...) Expand 10 before | Expand all | Expand 10 after
2874 Browser* browser = new Browser(params); 2910 Browser* browser = new Browser(params);
2875 gfx::Rect bounds = browser->window()->GetBounds(); 2911 gfx::Rect bounds = browser->window()->GetBounds();
2876 2912
2877 // Should be EXPECT_EQ, but this width is inconsistent across platforms. 2913 // Should be EXPECT_EQ, but this width is inconsistent across platforms.
2878 // See https://crbug.com/567925. 2914 // See https://crbug.com/567925.
2879 EXPECT_GE(bounds.width(), 100); 2915 EXPECT_GE(bounds.width(), 100);
2880 EXPECT_EQ(122, bounds.height()); 2916 EXPECT_EQ(122, bounds.height());
2881 browser->window()->Close(); 2917 browser->window()->Close();
2882 } 2918 }
2883 } 2919 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698