Revert "Don't use NOTIFICATION_BROWSER_CLOSE_CANCELLED in tests"

This reverts commit 7a5df8d5.

Reason for revert: Suspect it introduces timeout https://crbug.com/990692

DevToolsSanityTest.TestDeviceEmulation is reliably timing out with
"[FAILED] Timeout exceeded: 20 sec".

Original change's description:
> Don't use NOTIFICATION_BROWSER_CLOSE_CANCELLED in tests
> 
> Bug: 268984
> Change-Id: Ie41cb913d29ad690b9364d9c478c7a3c4771f607
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1730591
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#683662}

TBR=avi@chromium.org,estade@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 268984, 990692
Change-Id: Ia1213f589b2cf3b966381886f2d60ff646ae7103
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1734735Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683935}

chrome/browser/devtools/devtools_sanity_browsertest.cc

@@ -43,7 +43,6 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
-#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -787,21 +786,22 @@ IN_PROC_BROWSER_TEST_F(DevToolsBeforeUnloadTest,
       DevToolsWindowTesting::Get(windows[2])->main_web_contents());
   // Try to close second devtools.
   {
+    content::WindowedNotificationObserver cancel_browser(
+        chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED,
+        content::NotificationService::AllSources());
     chrome::CloseWindow(DevToolsWindowTesting::Get(windows[1])->browser());
     CancelModalDialog();
-    base::RunLoop().RunUntilIdle();
-    // The second devtools hasn't closed.
-    EXPECT_EQ(windows[1],
-              DevToolsWindow::GetInstanceForInspectedWebContents(
-                  DevToolsWindowTesting::Get(windows[0])->main_web_contents()));
+    cancel_browser.Wait();
   }
   // Try to close browser window.
   {
+    content::WindowedNotificationObserver cancel_browser(
+        chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED,
+        content::NotificationService::AllSources());
     chrome::CloseWindow(browser());
     AcceptModalDialog();
     CancelModalDialog();
-    base::RunLoop().RunUntilIdle();
-    EXPECT_EQ(browser(), BrowserList::GetInstance()->get(0));
+    cancel_browser.Wait();
   }
   // Try to exit application.
   {

chrome/browser/lifetime/browser_close_manager_browsertest.cc

@@ -77,14 +77,12 @@ app_modal::NativeAppModalDialog* GetNextDialog() {
 // trying to close it, to avoid flakiness. https://crbug.com/519646
 void AcceptClose() {
   GetNextDialog()->AcceptAppModalDialog();
-  base::RunLoop().RunUntilIdle();
 }
 
 // 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();
-  base::RunLoop().RunUntilIdle();
 }
 
 class RepeatedNotificationObserver : public content::NotificationObserver {
@@ -306,7 +304,7 @@ class BrowserCloseManagerBrowserTest : public InProcessBrowserTest {
   }
 
   void WaitForAllBrowsersToClose() {
-    while (!BrowserList::GetInstance()->empty())
+    for (size_t i = 0U; i < browsers_.size(); ++i)
       ui_test_utils::WaitForBrowserToClose();
   }
 
@@ -318,13 +316,16 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest, TestSingleTabShutdown) {
       browser(), embedded_test_server()->GetURL("/beforeunload.html")));
   PrepareForDialog(browser());
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
 
   chrome::CloseAllBrowsersAndQuit();
-  GetNextDialog()->AcceptAppModalDialog();
+  ASSERT_NO_FATAL_FAILURE(AcceptClose());
   ui_test_utils::WaitForBrowserToClose();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(BrowserList::GetInstance()->empty());
@@ -336,15 +337,18 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
       browser(), embedded_test_server()->GetURL("/beforeunload.html")));
   PrepareForDialog(browser());
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
   chrome::CloseAllBrowsersAndQuit();
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
 
   chrome::CloseAllBrowsersAndQuit();
   chrome::CloseAllBrowsersAndQuit();
-  GetNextDialog()->AcceptAppModalDialog();
+  ASSERT_NO_FATAL_FAILURE(AcceptClose());
   ui_test_utils::WaitForBrowserToClose();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(BrowserList::GetInstance()->empty());
@@ -358,8 +362,11 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest, PRE_TestSessionRestore) {
       ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAboutURL)));
   PrepareForDialog(browser());
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
 
   browser()->tab_strip_model()
@@ -409,8 +416,11 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest, TestMultipleWindows) {
 
   // Cancel shutdown on the first beforeunload event.
   {
+    RepeatedNotificationObserver cancel_observer(
+        chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
     chrome::CloseAllBrowsersAndQuit();
     ASSERT_NO_FATAL_FAILURE(CancelClose());
+    cancel_observer.Wait();
   }
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browsers_[0]->tab_strip_model()->count());
@@ -418,9 +428,12 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest, TestMultipleWindows) {
 
   // Cancel shutdown on the second beforeunload event.
   {
+    RepeatedNotificationObserver cancel_observer(
+        chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
     chrome::CloseAllBrowsersAndQuit();
     ASSERT_NO_FATAL_FAILURE(AcceptClose());
     ASSERT_NO_FATAL_FAILURE(CancelClose());
+    cancel_observer.Wait();
   }
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browsers_[0]->tab_strip_model()->count());
@@ -452,14 +465,17 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   // the dialog is guaranteed to show.
   PrepareForDialog(browsers_[0]->tab_strip_model()->GetWebContentsAt(1));
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   // All tabs should still be open.
   EXPECT_EQ(3, browsers_[0]->tab_strip_model()->count());
 
   chrome::CloseAllBrowsersAndQuit();
-  GetNextDialog()->AcceptAppModalDialog();
+  ASSERT_NO_FATAL_FAILURE(AcceptClose());
   ui_test_utils::WaitForBrowserToClose();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(BrowserList::GetInstance()->empty());
@@ -482,8 +498,11 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   // the dialog is guaranteed to show.
   PrepareForDialog(browsers_[1]);
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   // All windows should still be open.
   EXPECT_EQ(1, browsers_[0]->tab_strip_model()->count());
@@ -527,8 +546,11 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   PrepareForDialog(
       browsers_[0]->tab_strip_model()->GetWebContentsAt(kResposiveTabIndex));
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
 
   // All tabs should still be open.
@@ -536,7 +558,7 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
 
   // Quit, this time accepting close confirmation dialog.
   chrome::CloseAllBrowsersAndQuit();
-  GetNextDialog()->AcceptAppModalDialog();
+  ASSERT_NO_FATAL_FAILURE(AcceptClose());
   ui_test_utils::WaitForBrowserToClose();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(BrowserList::GetInstance()->empty());
@@ -557,7 +579,7 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
 IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
                        MAYBE_TestBeforeUnloadMultipleSlowWindows) {
   const int kBrowserCount = 5;
-  const int kResponsiveBrowserIndex = 2;
+  const int kResposiveBrowserIndex = 2;
   // Create multiple browsers with all tabs except one responding after
   // RenderViewHostImpl::kUnloadTimeoutMS .
   // Minimum configuration is just one browser with slow tab and then
@@ -569,16 +591,19 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
       browsers_.push_back(CreateBrowser(browser()->profile()));
     ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
         browsers_[i],
-        embedded_test_server()->GetURL((i == kResponsiveBrowserIndex)
+        embedded_test_server()->GetURL((i == kResposiveBrowserIndex)
                                            ? "/beforeunload.html"
                                            : "/beforeunload_slow.html")));
   }
   // Disable the hang monitor in the tab that is not expected to hang, so that
   // the dialog is guaranteed to show.
-  PrepareForDialog(browsers_[kResponsiveBrowserIndex]);
+  PrepareForDialog(browsers_[kResposiveBrowserIndex]);
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, kResposiveBrowserIndex + 1);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
 
   // All windows should still be open.
@@ -623,6 +648,8 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
       browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
   PrepareForDialog(browsers_[0]);
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
   chrome::CloseAllBrowsersAndQuit();
   browsers_.push_back(CreateBrowser(browser()->profile()));
   ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
@@ -630,6 +657,7 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   PrepareForDialog(browsers_[1]);
   ASSERT_NO_FATAL_FAILURE(AcceptClose());
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browsers_[0]->tab_strip_model()->count());
   EXPECT_EQ(1, browsers_[1]->tab_strip_model()->count());
@@ -677,6 +705,8 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   PrepareForDialog(browsers_[0]);
   PrepareForDialog(browsers_[1]);
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
   chrome::CloseAllBrowsersAndQuit();
   ASSERT_NO_FATAL_FAILURE(AcceptClose());
   AddBlankTabAndShow(browsers_[0]);
@@ -689,6 +719,7 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   PrepareForDialog(browsers_[1]);
   ASSERT_NO_FATAL_FAILURE(AcceptClose());
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(2, browsers_[0]->tab_strip_model()->count());
   EXPECT_EQ(2, browsers_[1]->tab_strip_model()->count());
@@ -752,7 +783,7 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   chrome::CloseWindow(browser2);
   // Just to be sure CloseWindow doesn't have asynchronous tasks
   // that could have an impact.
-  base::RunLoop().RunUntilIdle();
+  content::RunAllPendingInMessageLoop();
 
   // Closing browser shouldn't happen because of beforeunload handler.
   EXPECT_EQ(2u, BrowserList::GetInstance()->size());
@@ -767,11 +798,14 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
                   ->NeedToFireBeforeUnload());
   // Accept closing the first tab.
   ASSERT_NO_FATAL_FAILURE(AcceptClose());
+  // Just to be sure accepting a dialog doesn't have asynchronous tasks
+  // that could have an impact.
+  content::RunAllPendingInMessageLoop();
   // It shouldn't close the whole window/browser.
   EXPECT_EQ(2u, BrowserList::GetInstance()->size());
   EXPECT_EQ(2, browser2->tab_strip_model()->count());
   // Accept closing the second tab.
-  GetNextDialog()->AcceptAppModalDialog();
+  ASSERT_NO_FATAL_FAILURE(AcceptClose());
   ui_test_utils::WaitForBrowserToClose();
   // Now the second window/browser should be closed.
   EXPECT_EQ(1u, BrowserList::GetInstance()->size());
@@ -799,6 +833,8 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
       browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
   PrepareForDialog(browsers_[0]);
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
   chrome::CloseAllBrowsersAndQuit();
 
   browsers_.push_back(CreateBrowser(browser()->profile()));
@@ -808,6 +844,7 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   browsers_[1]->tab_strip_model()->CloseAllTabs();
   ASSERT_NO_FATAL_FAILURE(CancelClose());
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browsers_[0]->tab_strip_model()->count());
   EXPECT_EQ(1, browsers_[1]->tab_strip_model()->count());
@@ -828,6 +865,8 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
       browsers_[0], embedded_test_server()->GetURL("/beforeunload.html")));
   PrepareForDialog(browsers_[0]);
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 2);
   chrome::CloseAllBrowsersAndQuit();
 
   browsers_.push_back(CreateBrowser(browser()->profile()));
@@ -837,6 +876,7 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   ASSERT_FALSE(browsers_[1]->ShouldCloseWindow());
   ASSERT_NO_FATAL_FAILURE(CancelClose());
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browsers_[0]->tab_strip_model()->count());
   EXPECT_EQ(1, browsers_[1]->tab_strip_model()->count());
@@ -861,10 +901,13 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
   PrepareForDialog(browsers_[0]);
   PrepareForDialog(browsers_[1]);
 
+  RepeatedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED, 1);
   chrome::CloseAllBrowsersAndQuit();
 
   ASSERT_FALSE(browsers_[0]->ShouldCloseWindow());
   ASSERT_NO_FATAL_FAILURE(CancelClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   EXPECT_EQ(1, browsers_[0]->tab_strip_model()->count());
   EXPECT_EQ(1, browsers_[1]->tab_strip_model()->count());
@@ -1096,9 +1139,13 @@ IN_PROC_BROWSER_TEST_F(BrowserCloseManagerBrowserTest,
       browser(), embedded_test_server()->GetURL("/beforeunload.html")));
   PrepareForDialog(browser());
 
+  content::WindowedNotificationObserver cancel_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED,
+      content::NotificationService::AllSources());
   TestBrowserCloseManager::AttemptClose(
       TestBrowserCloseManager::USER_CHOICE_USER_CANCELS_CLOSE);
   ASSERT_NO_FATAL_FAILURE(AcceptClose());
+  cancel_observer.Wait();
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
 
   TestBrowserCloseManager::AttemptClose(