Wait for OnBrowserRemoved after closing window in tests

Some IndependentOTRProfileManager tests need to wait for a browser
window and its associated Browser object to be destroyed.  This was
originally done with base::RunLoop::RunUntilIdle, but this isn't
necessarily sufficient and may be a source of flakes on Mac.

Bug: 835834, 837236
Change-Id: I8dfd71e45cea7df411693dbccb153f9554754809
Reviewed-on: https://chromium-review.googlesource.com/1034279Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Brandon Tolsch <btolsch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554874}

chrome/browser/media/router/presentation/independent_otr_profile_manager_browsertest.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -67,6 +68,30 @@ class ProfileDestructionWatcher final : public content::NotificationObserver {
   content::NotificationRegistrar registrar_;
 };
 
+// Waits for |browser| to be removed from BrowserList and then calls |callback|.
+// This is used to ensure that the Browser object and its window are destroyed
+// after a call to BrowserWindow::Close, since base::RunLoop::RunUntilIdle
+// doesn't ensure this on Mac.
+class BrowserRemovedWaiter final : public BrowserListObserver {
+ public:
+  BrowserRemovedWaiter(Browser* browser, base::OnceClosure callback)
+      : browser_(browser), callback_(std::move(callback)) {
+    BrowserList::AddObserver(this);
+  }
+  ~BrowserRemovedWaiter() override = default;
+
+  void OnBrowserRemoved(Browser* browser) override {
+    if (browser == browser_) {
+      BrowserList::RemoveObserver(this);
+      std::move(callback_).Run();
+    }
+  }
+
+ private:
+  Browser* browser_;
+  base::OnceClosure callback_;
+};
+
 void OriginalProfileNeverDestroyed(Profile* profile) {
   FAIL()
       << "Original profile unexpectedly destroyed before dependent OTR profile";
@@ -272,16 +297,8 @@ IN_PROC_BROWSER_TEST_F(IndependentOTRProfileManagerTest,
   EXPECT_TRUE(destroyed2);
 }
 
-// Flaky on mac, crbug.com/837236
-#if defined(OS_MACOSX)
-#define MAYBE_BrowserClosingDoesntRemoveProfileObserver \
-  DISABLED_BrowserClosingDoesntRemoveProfileObserver
-#else
-#define MAYBE_BrowserClosingDoesntRemoveProfileObserver \
-  BrowserClosingDoesntRemoveProfileObserver
-#endif
 IN_PROC_BROWSER_TEST_F(IndependentOTRProfileManagerTest,
-                       MAYBE_BrowserClosingDoesntRemoveProfileObserver) {
+                       BrowserClosingDoesntRemoveProfileObserver) {
   ProfileDestructionWatcher watcher1;
   ProfileDestructionWatcher watcher2;
   bool destroyed1 = false;
@@ -306,8 +323,11 @@ IN_PROC_BROWSER_TEST_F(IndependentOTRProfileManagerTest,
     otr_browser = CreateBrowser(otr_profile2);
     watcher2.Watch(otr_profile2, &destroyed2);
   }
+  base::RunLoop run_loop;
+  BrowserRemovedWaiter removed_waiter(otr_browser,
+                                      run_loop.QuitWhenIdleClosure());
   otr_browser->window()->Close();
-  base::RunLoop().RunUntilIdle();
+  run_loop.Run();
   ASSERT_FALSE(base::ContainsValue(*BrowserList::GetInstance(), otr_browser));
   EXPECT_TRUE(destroyed2);
 
@@ -317,15 +337,8 @@ IN_PROC_BROWSER_TEST_F(IndependentOTRProfileManagerTest,
   EXPECT_TRUE(destroyed1);
 }
 
-// Flaky on mac, crbug.com/835834
-#if defined(OS_MACOSX)
-#define MAYBE_CallbackNotCalledAfterUnregister \
-  DISABLED_CallbackNotCalledAfterUnregister
-#else
-#define MAYBE_CallbackNotCalledAfterUnregister CallbackNotCalledAfterUnregister
-#endif
 IN_PROC_BROWSER_TEST_F(IndependentOTRProfileManagerTest,
-                       MAYBE_CallbackNotCalledAfterUnregister) {
+                       CallbackNotCalledAfterUnregister) {
   ProfileDestructionWatcher watcher;
   Browser* otr_browser = nullptr;
   Profile* otr_profile = nullptr;
@@ -338,8 +351,11 @@ IN_PROC_BROWSER_TEST_F(IndependentOTRProfileManagerTest,
   }
   bool destroyed = false;
   watcher.Watch(otr_profile, &destroyed);
+  base::RunLoop run_loop;
+  BrowserRemovedWaiter removed_waiter(otr_browser,
+                                      run_loop.QuitWhenIdleClosure());
   otr_browser->window()->Close();
-  base::RunLoop().RunUntilIdle();
+  run_loop.Run();
   ASSERT_FALSE(base::ContainsValue(*BrowserList::GetInstance(), otr_browser));
   EXPECT_TRUE(destroyed);
 }